-
Notifications
You must be signed in to change notification settings - Fork 74
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
🦋Update provider account Invitations index page #3923
base: master
Are you sure you want to change the base?
Conversation
flash[:success] = 'Invitation will be resent soon.' | ||
redirect_to provider_admin_account_invitations_path | ||
end | ||
format.xml { head :ok } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure this is not used anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, not really but it was added 10+ years ago and I think XML responses are only used by API endpoints.
There is nothing about it in 3scale API docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UPDATE: After some digging I can't really justify removing this response so I will restore it and add a comment for posterity.
3e8332c
to
3d3d5af
Compare
# def sent_date(invitation) | ||
# invitation.sent_at&.to_s(:long) || 'Not sent yet' | ||
# end | ||
|
||
# def status(invitation) | ||
# invitation_status(invitation) | ||
# # if invitation.accepted? | ||
# # "yes, on #{invitation.accepted_at.to_s(:short)}" | ||
# # else | ||
# # 'no' | ||
# # end | ||
# end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better don't push commented code IMO
# end | ||
|
||
def can_manage_invitation?(invitation) | ||
@can_manage_invitation ||= @ability.can?(:manage, invitation) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks wrong, after the first call, it will cache the value of @ability.can?(:manage, invitation)
and return it on subsequent calls even when called with different invitation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would actually work in this case, because for the provider users the abilities are for the whole class, there is no condition that would make it different for different invitations - it's either allowed for all, or not allowed for all.
But I agree that still it's not right 😬
@@ -1,3 +1,6 @@ | |||
# frozen_string_literal: true | |||
|
|||
# TODO: remove and use presenter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there are only two small methods left here, why not move them to the presenter directly instead of leaving a TODO?
THREESCALE-9883: Add a toolbar to the table
### Before
After