Fix card grant settings action visibility - #14686
Conversation
|
(pulled the latest changes) |
|
Hi, this button was meant to be accessible to members so they could see the settings forms in a disabled state. I think this worked in the past but I just checked to confirm it is broken. Could you please update this PR so it still renders the button but then fix the button so it opens the settings form with the fields disabled? |
|
Alright, I'll update it. |
|
|
||
| def edit_overview | ||
| authorize @card_grant | ||
| authorize @card_grant, :show? |
There was a problem hiding this comment.
Show allows the cardholder to access this. I think we should update the edit policy to be auditor_or_member? and then alias the other edit policies to show.
| <section class="modal modal--scroll modal--wide bg-snow" data-behavior="modal" role="dialog" id="edit_card_grant"> | ||
| <%= modal_header(policy(card_grant).edit? ? "Manage grant" : "Grant settings") %> | ||
| <%= turbo_frame_tag "edit_#{card_grant.id}", src: edit_overview_event_card_grant_path(card_grant) %> | ||
| </section> |
There was a problem hiding this comment.
This should still be wrapped in an if but the link_to that points to it also needs an if. The reason this bug popped up was because we weren't rendering the modal but were rendering the button to access it.
There was a problem hiding this comment.
yep, I see now where that's wrong. I'll fix that
| describe "read-only grant settings" do | ||
| it "allows members to view, but not edit, grant settings" do | ||
| member = create(:user) | ||
| event = create(:event, :with_positive_balance, plan_type: Event::Plan::HackClubAffiliate) | ||
| create(:organizer_position, user: member, event:, role: :member) | ||
| card_grant = create(:card_grant, event:) | ||
| create_session(member, verified: true) | ||
|
|
||
| get(:edit_purpose, params: { event_id: event.friendly_id, id: card_grant.hashid }) | ||
|
|
||
| expect(response).to have_http_status(:ok) | ||
| expect(response.parsed_body.at_css("[name='card_grant[purpose]']")["disabled"]).to eq("disabled") | ||
| expect(response.parsed_body.at_css("[value='Save']")["disabled"]).to eq("disabled") | ||
| end | ||
| end |
There was a problem hiding this comment.
This is a good test of the UI but it may be more verbose to also test calling update as a member and ensuring that it fails.
|
very small thing that fixed the tooltip, now if you can view grant settings the tooltip shows view settings as opposed to manage card grant. |
|
(update branch again) |
2 participants
Summary of the problem
initial help thread where i discovered it: slack thread
Card grant pages show a "View Grant Settings" button to all members of the organisation, regardless of if the user has the permission to edit grants.
If you are a member of the organisation but don't have permission to manage the grant, and you click on the button, you get
Uncaught Error: Missing target element "toggle" for "menu" controllerin the console, and no clear error notification.Describe your changes
I updated the card grant header so the settings action only appears for users who can actually access the grant settings UI. Previously, any organisation member could see the View grant settings button, even if they lacked edit_overview? permission, which led to a broken interaction.
Before:
After:
