Skip to content

Conversation

@jrafanie
Copy link
Member

@jrafanie jrafanie commented Nov 4, 2025

TODO:

  • Extract the Widget change for a followup that is based on the changes here.

  • Store these things in a different key such as :role_ids

  • Make sure any shared code in widgets / custom buttons are not broken by this change.

  • Research the comment regarding both string and int role ids in the @edit new/old arrays, one for old and new values.
    [ ] Write a data migration to convert existing CustomButton rows with visibility columns from:

    {:roles => ["EvmRole-super_administrator"]}
    

    To:

    {:roles => [1]}
    

    Or(see the first bullet):

    {:role_ids => [1]}
    

@jrafanie jrafanie requested a review from a team as a code owner November 4, 2025 21:33
@jrafanie jrafanie added the bug label Nov 4, 2025
edit[:new][:roles] = [old_role.id, role.id.to_s] # old roles are represented by int and new ones by string
controller.send(:button_set_record_vars, custom_button)
expect(custom_button.visibility[:roles]).to eq([old_role.name, role.name])
expect(custom_button.visibility[:roles]).to eq([old_role.id, role.id.to_s])
Copy link
Member Author

@jrafanie jrafanie Nov 4, 2025

Choose a reason for hiding this comment

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

TODO: that comment on line 347 is nuts. I need to figure out if roles as strings and ints is an actual thing.

roles.push(role.name) if role
end
button.visibility[:roles] = roles
button.visibility[:roles] = @edit[:new][:roles]
Copy link
Member Author

Choose a reason for hiding this comment

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

TODO: keep the lookup but drop the loop...

button.visibility[:roles] = MiqUserRole.where(:id => @edit[:new][:roles]).pluck(:id)

jrafanie added a commit to jrafanie/manageiq-ui-classic that referenced this pull request Nov 4, 2025
@jrafanie jrafanie force-pushed the fix-custom-buttons-using-role-names-that-can-change branch from 03d702e to 725a24b Compare November 4, 2025 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants