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

_navigation.html.erb: Hide resources without index #1524

Merged
merged 1 commit into from Jan 18, 2020

Conversation

bekicot
Copy link
Contributor

@bekicot bekicot commented Jan 13, 2020

This will hide link on the navigation bar for resources without an index page.

Add interface actions to Administrate::Namespace::Resource that can be a solution to #1517.

This is related to #1517 but not directly fix it.

@bekicot
Copy link
Contributor Author

bekicot commented Jan 13, 2020

@pablobm please review this one. It simpler problem than #1517.

@pablobm
Copy link
Collaborator

pablobm commented Jan 16, 2020

Thank you for this @bekicot! I think you are on the right track, but this perhaps could be simpler? Would it be possible to use the existing valid_action? instead of resource.actions.include?. See my comment at #1418 (review)

@bekicot
Copy link
Contributor Author

bekicot commented Jan 17, 2020

Thank you for this @bekicot! I think you are on the right track, but this perhaps could be simpler? Would it be possible to use the existing valid_action? instead of resource.actions.include?. See my comment at #1418 (review)

@pablobm I think valid action only work on the page that the action is available.
e.g:
suppose we usevalid_action? :index to show orders admin link in navigation.

- on products page /admin/products && /admin/products/:id will work, since ProductsController have have :index . And Product link will show on navigation.

- on line items page /admin/product_meta_tags && /admin/product_meta_tags/:id wont work, since ProductMetaTagsController doesn't have :index, Product link wont show on navigation


After re-read your comment properly at #1418, I think I have to support my assumtion that valid_action? :index, resource wont work on this particular problem.


Above comment was made with premature knowledge.

@pablobm
Copy link
Collaborator

pablobm commented Jan 17, 2020

Sorry, I don't understand your example :-( Did you mean "products" when you said "orders"?

In your spec, you make Product Meta Tags not have an index page. Therefore it doesn't show on the navigation. I expect that it will not show in the navigation in any page: it will not show in products/index, products/show, product_meta_tags/show, etc.

The navigation partial is the same in all pages, regardless of what resource we are looking at.

Also, I have tried implementing it with if valid_action?(:index, resource), and it does what I expect. Perhaps your expectation is different?

@bekicot
Copy link
Contributor Author

bekicot commented Jan 18, 2020

I was wrong, I wasn't carefully read the source code and made some wrong assumptions.

Looks like #1418 is aiming toward to fix the same issue. If you prefer to wait on that PR to be fixed, you can close this one since this PR is later.

If you think you prefer this PR and request some change, let me know.

@pablobm
Copy link
Collaborator

pablobm commented Jan 18, 2020

I think that your solution was much closer, and it will be easier for you to adapt it to what I'm asking. If you do it, I'll approve this PR.

@bekicot
Copy link
Contributor Author

bekicot commented Jan 18, 2020

I think that your solution was much closer, and it will be easier for you to adapt it to what I'm asking. If you do it, I'll approve this PR.

I've already used valid_action? :index, resource instead in this commit 8f21f7b.

@pablobm
Copy link
Collaborator

pablobm commented Jan 18, 2020

Oh sorry, I didn't notice you had pushed changes!

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

2 participants