Add/Remove CustomButton(s) to CustomButtonSet

[ZitaNemeckova]

It's impossible to add/remove a CustomButton to/from CustomButtonSet via API but it's possible to change set_data[:button_order] that only says in what order buttons go. It would good to add it as well so UI can only call API and not use extra http request to do it :)

@miq-bot assign @skateman

Related to ManageIQ/manageiq-ui-classic#4913

[skateman]

I see two possible ways how this can be added to the API:

• custom_buttons as a subcollection of custom_button_sets, IMO this doesn't really make sense because a button can be also unassigned
• passing a custom_button_set_id when creating/editing a custom_button and doing the necessary db changes in the Api::CustomButtonsController under create_resource and edit_resource

I vote for the 2nd option, but I might need some help with error handling.

[ZitaNemeckova]

@skateman Second option means that if a user adds/removes multiple buttons when editing Custom Button Group there's gonna be one API call per button, right?

[skateman]

@ZitaNemeckova yes

[skateman]

Maybe there are other solutions that I'm not aware of, waiting for the API people to show up.

cc @gtanzillo @abellotti

[skateman]

Another idea thanks to @himdel: having a before_save callback on the CustomButtonSet models that maps button_order to the members= method:

  before_save :update_members
  def update_members
    self.members = CustomButton.find(set_data[:button_order])
  end

I really like this idea, but I don't want to introduce asymmetry with other parts of the API that I don't know 😟 so it would be really nice if I'd have some input on these options from someone who understands the codebase a little better.

[djberg96]

I guess I don't see the issue with making custom_button a subcollection of custom_button_sets. If you need to create an unassigned button, don't use the custom_button_set api. With it as a subcollection we would get this:

GET /api/custom_button_sets/:c_id/custom_buttons
GET /api/custom_button_sets/:c_id/custom_button/:s_id
POST /api/custom_button_sets/:c_id/custom_button/:s_id { "action": "delete" }
# and so on

[himdel]

My concern is that we currently have 2 independent places where we list the custom buttons associated with the set (one as an array of ids in button_order, and the other as an unordered(? - my assumption) set in members (or children)).

If we had a nice way of having our subcollection, and custom-ordering it too, without that duplication, it would be perfect :).

(That said, as long as it is possible to manipulate both lists via the API, we're mostly fine (except for potential short-term bugs caused by non-atomicity of that approach).)

[gtanzillo]

@abellotti and I looked at this today

We were thinking that you could have an action on custom_button_sets that allows for assigning one or more buttons to the set

        POST /api/custom_button_sets/:id

                actions -> "assign_custom_buttons"
                        
                        "resources" : [
                                { "href" : "/api/custom_buttons/1" },
                                { "id" : "2" },
                                { "href" : "/api/custom_buttons/3" },
                        ]

                actions -> "unassign_custom_buttons"

                        "resources" : [
                                { "href" : "/api/custom_buttons/1" },
                                { "id" : "2" },
                        ]

In addition to that we were thinking that it would also be good to allow assigning all the buttons in the set on the create and edit

POST /api/custom_button_sets                    - create

        {
                data ....,
                "custom_buttons" : [
                        { "href" : "/api/custom_buttons/1" },
                        { "id" : "2" },
                        { "href" : "/api/custom_buttons/3" }
                ]
        }


POST /api/custom_button_sets                    - edit


        {
                "action" : "edit",
                "resource" : {
                        "custom_buttons" : [
                                { "href" : "/api/custom_buttons/1" },
                                { "id" : "2" },
                                { "href" : "/api/custom_buttons/3" }
                        ]
                }
        }

[skateman]

@gtanzillo the important part not to miss is the button order, just saying so we won't forget about it.

[himdel]

Created ManageIQ/manageiq#18368 , please review :)

We don't actually need those add button/remove button actions, ever .. we only ever send the final list of buttons to the set.
And since we have to do that, as that's the only place keeping the ordering, we might as well use the info to create the member list properly :).