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

hasMany collection columns #2415

Merged
merged 12 commits into from
Oct 18, 2023
Merged

hasMany collection columns #2415

merged 12 commits into from
Oct 18, 2023

Conversation

jubilee2
Copy link
Contributor

@jubilee2 jubilee2 commented Aug 9, 2023

idea come from nickcharlton/administrate-field-nested_has_many#54 but I use positive list not only skip the column but also can add/remove/order the columns.

@jubilee2 jubilee2 changed the title Patch 4 hasMany collection columns Aug 9, 2023
Copy link
Collaborator

@pablobm pablobm left a comment

Choose a reason for hiding this comment

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

Looks good! 🙂 Would you be able to add a test? I'm thinking a simple one would be to change the order dashboards to display the line items like this:

    line_items: Field::HasMany.with_options(collection_attributes: [:product, :quantity, :unit_price, :total_price]),

Then you can check that the "Order" column doesn't appear. What do you think?

@jubilee2
Copy link
Contributor Author

Looks good! 🙂 Would you be able to add a test? I'm thinking a simple one would be to change the order dashboards to display the line items like this:

    line_items: Field::HasMany.with_options(collection_attributes: [:product, :quantity, :unit_price, :total_price]),

Then you can check that the "Order" column doesn't appear. What do you think?

added

@pablobm
Copy link
Collaborator

pablobm commented Oct 17, 2023

Oof, build issue 😫 I think the issue is related to #2410. Would you be able to rebase on top of the latest main and re-push?

@jubilee2
Copy link
Contributor Author

Oof, build issue 😫 I think the issue is related to #2410. Would you be able to rebase on top of the latest main and re-push?

merged main

@pablobm
Copy link
Collaborator

pablobm commented Oct 18, 2023

OK, let's go! 🚀

Thank you for the contribution 🙂

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.

2 participants