Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deep nested modules #775

Merged
merged 7 commits into from
Apr 26, 2021
Merged

Deep nested modules #775

merged 7 commits into from
Apr 26, 2021

Conversation

liamjcooper
Copy link
Contributor

This PR allows deep nesting for submodules. I am not sure if this is something that is highly requested, I could not find a similar PR or issue regarding this. Currently, Twill allows a single nested module, but it's possible a project may require further nesting as I have found. Consider the following modules and their relationships:

  • client
  • clients have many projects (intranet, reporting portal, customer portal etc)
  • projects have many applications (e.g. website, mobile application etc)

So, as Laravel nested resources allow, you could have a route as such:
/clients/3/projects/5/applications/2

I have made the required changes however it might need some extra testing? I tried it out on a project I'm using with Twill and I could get these changes to work regarding routing, creating, editing, deleting, etc. I am fairly new to contributing to open source so please bear with me if you need more coverage. I am also new to unit testing, so if anyone could help me out in this regard that would be much appreciated, if required.

The syntax is essentially how the docs portray a regular submodule, you just add the extra modules in the nest with dot notation, for example:

// ClientProjectApplicationController.php
<?php

namespace App\Http\Controllers\Admin;

use A17\Twill\Http\Controllers\Admin\ModuleController;

class ClientProjectApplicationController extends ModuleController
{
    protected $modelName = 'Application';
    protected $moduleName = 'clients.projects.applications';
}
<?php
Route::module('clients');
Route::module('clients.projects');
Route::module('clients.projects.applications');

And then views also take the same breadth:
resources/views/admin/clients/projects/applications

A lot of the changes to support this deep nesting lies in the arguments for controller actions - as the method for retrieving an ID depended on there being strictly 1-2 parameters coming from the route. This is limiting because this assumes there would only ever be 1 level nested. Now, these controller actions depend on the Illuminate\Http\Request class to gather the route and its array of parameters to predictably determine the nesting level and its parents ID. If this imposes a security risk I am not aware of, please let me know, and we will work to a better solution.

@ifox
Copy link
Member

ifox commented Mar 26, 2021

/rebase

@ptrckvzn ptrckvzn force-pushed the deep-nested-modules branch from 09d017e to 88e336b Compare April 7, 2021 22:14
@ifox ifox force-pushed the deep-nested-modules branch from 64ab265 to 4c3e573 Compare April 26, 2021 16:34
@ifox ifox merged commit 7fefd20 into area17:2.x Apr 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants