Hide link to delete users if they have orders #3139
Conversation
There was a problem hiding this comment.
Helpful UX change.
I think in the future we may consider a more complete solution that complies with GDPR requests by removing any PII for a given user extending what is done in solidus_auth_devise https://github.com/solidusio/solidus_auth_devise/blob/master/app/models/spree/user.rb#L10
There was a problem hiding this comment.
I am OK with this until we reach a resolution to #3138, but that should be a priority.
aitbw
force-pushed
the
aitbw/avoid-user-destruction
branch
from
101c9a4 to
2e96c7a
Compare
|
@aitbw thanks! |
| @@ -89,7 +89,7 @@ | |||
| <% if can?(:edit, user) %> | |||
| <%= link_to_edit user, no_text: true %> | |||
| <% end %> | |||
| <% if can?(:destroy, user) %> | |||
| <% if can?(:destroy, user) && user.orders.count.zero? %> | |||
There was a problem hiding this comment.
what is the advantage of doing user.orders.count.zero? instead of user.orders.empty?
There was a problem hiding this comment.
None, but since both snippets generate a SQL query, they're pretty much exchangeable. Your suggestion is less verbose and feels more natural, though.
|
Hi @kainio, sorry for the late reply —yes, you're correct; this PR aims to serve as a workaround. Since the bug reported in #3126 only affects the backend side of Solidus, I think it's safe to assume admins won't be trying to hack their own stores 😸 I personally am working on a proper fix for the aforementioned issue, but we still haven't reached consensus on #3138 —feel free to join the discussion 🙂 |
kennyadsl
changed the title
Description
This PR hides the
deletebutton for users that have at least one order associated to their profiles, which aims to serve as a starting point to solve #3126.Ref: #3138
Checklist: