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

Super Scaffold with custom namespaces properly #242

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

gazayas
Copy link
Contributor

@gazayas gazayas commented Apr 11, 2023

Continuation of #35.

Example model

> rails g model Job team:references title:string
> bin/super-scaffold crud Job Team title:text_field --namespace=client

Details

In #35 we got the routes to work properly, but there's still some more work to do here.

I was able to scaffold the API controller (which I believe we'll need because it has all of our strong parameters), and I namespaced the class names of the controllers.

However, I'm getting this error and I'm not quite sure how to handle it from here:
Screenshot from 2023-04-11 21-34-51

@gazayas
Copy link
Contributor Author

gazayas commented Apr 12, 2023

Yeah, the more I'm thinking through this one the more it looks like it's going to take some work. We'll need to:

  1. Move the model and namespace it
  2. Redo the routes so they're scoped under Team
  3. The regular views are fine, but we need to make a namespaced directory for the jbuilder files
  4. Update the contents of the controllers so identifiers match the model (i.e. - :client_job as opposed to :job).

I'm not quite sure I have the solution for this one yet though, so I'll need to work with it to see what I can do. I'm somewhat reluctant to move forward thought because I know there are many namespacing options in the Bullet Train blog, and I want to make sure I'm going with the right one. Probably might benefit to make a Scaffolding::Namespacer class since some of the logic repeats itself too.

Routing

Right now, the routes look like this:

begin
  namespace :client do
    shallow do
      resources :teams do
        resources :jobs
      end
    end
  end
end

@gazayas
Copy link
Contributor Author

gazayas commented Apr 12, 2023

We can definitely just do this if we want to namespace a model:

> rails g model Client::Job team:references title:string
> bin/super-scaffold crud Client::Job Team title:text_field

For that reason, I'll leave this one as is until further direction concerning what file structure, etc. we want for using --namespace.

@jagthedrummer
Copy link
Contributor

@gazayas what are your current thoughts on this one? It seems like maybe we don't need it if you can namespace the model directly, as you mentioned?

@gazayas
Copy link
Contributor Author

gazayas commented Aug 9, 2023

@jagthedrummer I think I had a misunderstanding of the use of the --namespace flag, and now that I'm looking at the docs, the purpose is to replace the Account namespace. Looking at the error in the initial comment in this PR, we're getting Account::Client::JobsController which isn't right, so I'm going to have to work on this one some more.

Linking bullet-train-co/bullet_train-super_scaffolding#78

@gazayas
Copy link
Contributor Author

gazayas commented Aug 17, 2023

After thinking over this one some more, from a design aspect I think I would need some more clarity before moving forward with this one.

For example, this is the scaffolding command from the documentation:

bin/super-scaffold crud Event Team name:text_field --namespace=customers

According to the documentation, the purpose of using the custom namespace is to expose these records so that they're public-facing. You can see here that the resource still belongs to a Team, resulting in routes like this (the routes don't work though because Customers::TeamsController doesn't exist):

--[ Route 20 ]----------------------------
Prefix            | customers_team_events
Verb              | GET
URI               | /customers/teams/:team_id/events(.:format)
Controller#Action | customers/events#index
--[ Route 21 ]----------------------------
Prefix            | 
Verb              | POST
URI               | /customers/teams/:team_id/events(.:format)
Controller#Action | customers/events#create
--[ Route 22 ]----------------------------
Prefix            | new_customers_team_event
Verb              | GET
URI               | /customers/teams/:team_id/events/new(.:format)
Controller#Action | customers/events#new

Also, I think this will be somewhat of a chore to update since this feature was last functionable when all of the Bullet Train logic existed in one repository (before we started using Tailwind??), but at the same time (if I'm understanding the purpose of it correctly) it would solve #251, and we should probably prioritize it anyways because it's written in the documentation. I can try to tread through this one, I just don't have a clear goal in sight from a code perspective, so I'm not entirely sure what the resulting PR would look like as of now.

@andrewculver pinging you for the following questions:

  1. Should we prioritize this feature sooner than later?
  2. Do you have any specific direction from a design perspective about how to handle this one?

@gazayas
Copy link
Contributor Author

gazayas commented Aug 17, 2023

Ah, it looks like public-facing resources should only exclusively fall under the Public namespace, and that the --namespace flag is specifically for organization purposes.

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.

None yet

3 participants