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

Fix scaffold template with namespace and --model-name option #2534

Merged
merged 6 commits into from Dec 24, 2021

Conversation

kenzo-tanaka
Copy link
Contributor

@kenzo-tanaka kenzo-tanaka commented Nov 19, 2021

close #2532

Fixed the behavior of scaffold template when namespace and --model-name option passed.

$ rails g scaffold_controller admin/products --model-name=product
describe "POST /create" do
  context "with valid parameters" do
    it "creates a new Product" do
      expect {
        # Here, I expected `{ product: valid_attributes }` like the test of update action
        post admin_products_url, params: { admin_product: valid_attributes } 
      }.to change(Product, :count).by(1)
    end
    # ...
end

This is because ns_file_name method that is called in scaffold template returns admin_product when namespaced.

def ns_file_name
return file_name if ns_parts.empty?
"#{ns_prefix.map(&:underscore).join('/')}_#{ns_suffix.singularize.underscore}"
end

So, I replaced ns_file_name to singular_table_name in scaffold templates.

Also, I added some tests to ensure that templates works in the old way when the --model-name option is not passed.

Replace ns_file_name to singular_table_name.
rspec#2532
@pirj
Copy link
Member

pirj commented Nov 19, 2021

Nice, thank you!

What happens if you don't specify --model-name=card in rails g scaffold_controller admin/cards, will it behave in the old way, will it use admin_cards (admin_products from the ticket's example)?

In case no specs fail, would you please add some examples to prevent from regression to spec/generators/rspec/scaffold/scaffold_generator_spec.rb?

@kenzo-tanaka
Copy link
Contributor Author

@pirj Thank you for quick response!

What happens if you don't specify --model-name=card in rails g scaffold_controller admin/cards, will it behave in the old way, will it use admin_cards (admin_products from the ticket's example)?

Thanks for pointing that out. If I remove --model-name=card, it behaved in the old way like below:

describe "POST /create" do
  context "with valid parameters" do
    it "creates a new Admin::Card" do
      expect {
        post admin_cards_url, params: { admin_card: valid_attributes }
      }.to change(Admin::Card, :count).by(1)
    end
   # ...
  end
end

Is this the behavior that you expected?


In case no specs fail, would you please add some examples to prevent from regression to spec/generators/rspec/scaffold/scaffold_generator_spec.rb?

OK! I'll try it.

@pirj
Copy link
Member

pirj commented Nov 19, 2021

Is this the behavior that you expected?

Yes, perfect. Thank you!

- check if parameter name is not admin_post
- check if test contains Post.create instead of Admin::Post.create

related:
rspec#2534 (comment)
@pirj pirj requested a review from JonRowe November 19, 2021 11:56
Copy link
Member

@pirj pirj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect, thank you!

@kenzo-tanaka
Copy link
Contributor Author

@pirj
When I run rails g scaffold_controller admin/cards --model-name=card --controller_specs in a example app, the controller spec may have a same situation of this issue.

describe "POST #create" do
  context "with valid params" do
    it "creates a new Card" do
      expect {
        post :create, params: {admin_card: valid_attributes}, session: valid_session
      }.to change(Card, :count).by(1)
    end
    # ...
  end
end

Should this file be fixed as well?

post :create, params: {<%= ns_file_name %>: valid_attributes}, session: valid_session

@pirj
Copy link
Member

pirj commented Nov 19, 2021

It would be great if you're up for it.

@kenzo-tanaka
Copy link
Contributor Author

@pirj OK! I'll try it tomorrow 😄

@PedroAugustoRamalhoDuarte
Copy link
Contributor

We have to change templates/api_request_spec.rb too, there is the same problem.

@kenzo-tanaka kenzo-tanaka changed the title Fix ns_file_name to singular_table_name in lib/generators/rspec/scaffold/templates/request_spec.rb. Fix scaffold template with namespace and --model-name option Nov 20, 2021
@kenzo-tanaka
Copy link
Contributor Author

@pirj @PedroAugustoRamalhoDuarte
Finally, I fixed the following files because they had the same problem.

  • templates/request_spec.rb
  • templates/api_request_spec.rb
  • templates/controller_spec.rb
  • templates/api_controller_spec.rb

And also, I added some examples to prevent from regression. Please check🙏

Copy link
Member

@pirj pirj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job! Thanks a lot for taking care of the whole thing.

@JonRowe JonRowe merged commit da1ec51 into rspec:main Dec 24, 2021
@JonRowe
Copy link
Member

JonRowe commented Dec 24, 2021

Thanks for tackling this

JonRowe added a commit that referenced this pull request Dec 24, 2021
JonRowe added a commit that referenced this pull request Jan 25, 2022
Fix scaffold template with namespace and `--model-name` option
JonRowe added a commit that referenced this pull request Jan 25, 2022
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.

Parameter name is not equal the model name when executing scaffold_controller with namespace
4 participants