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

Do not show broken links in admin product view when product is deleted #1988

Merged
merged 1 commit into from
Jun 12, 2017

Conversation

andoneve
Copy link
Contributor

@andoneve andoneve commented Jun 2, 2017

Fixes #1987 - Admin product page for a deleted item shows errors when clicking around.

Hides broken tabs (Images, Variants, Prices, Product Stock) in Admin Product View if the product is deleted.

@andoneve andoneve changed the title Do not show broken links in admin product view when product is deleted #1987 Do not show broken links in admin product view when product is deleted Jun 2, 2017
Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! I was thinking about wrapping all those elements into a single:

<% if !@product.deleted? %>

but your solution is probably better to keep elements modular.

Another option could be adding a redirect at controller level (that we probably should do anyway) but IMO from a UX point of view is much better to remove/disable those links.

Copy link
Contributor

@mamhoff mamhoff left a comment

Choose a reason for hiding this comment

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

I think this is a good-enough fix. For sure we would want to add redirects to the controllers, but I'm not sure people would actually try hitting those endpoints (as logged-in admin users...).

Another thought I had is that some of those tabs should actually work with deleted products, as their variants and prices are probably only soft-deleted, too. But that's much more UI work that is probably not worth it.

Thank you for your contribution!

@andoneve
Copy link
Contributor Author

andoneve commented Jun 3, 2017

Thanks for the comments/feedback.

I don't think it's worth adding the redirects in the controller unless you come across the problem (in real life) at some point in the future. This is really a small, dark, unvisited corner of the application.

@mamhoff As for looking more into the soft-deleted variants and prices (and why they aren't just there?), I think that's a good idea for the future.

Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

👍

@mamhoff
Copy link
Contributor

mamhoff commented Jun 7, 2017

The variant view has been fixed in #1804, so we can actually display that link. Would you mind removing the if condition there, and rebasing?

Fixes solidusio#1987 - Admin product page for a deleted item shows errors when clicking around.
Hides the broken tabs if the product is deleted. Broken: Images, Prices,
Product Properties and Product Stock.
@kennyadsl kennyadsl merged commit eae7bf7 into solidusio:master Jun 12, 2017
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.

4 participants