Fix cluster installation processing spinner

What does this MR do?

This fixes the cluster installation processing spinner on the installation buttons.

before	after

Solution

1. Button says "Install"
2. Click button
3. Button says "Installing" with spinner (greyed out)

What are the relevant issue numbers?

Closes #56398 (closed)

changed milestone to %11.8

added Deferred UX frontend maintenancerefactor typebug + 1 deleted label

added 1 commit

• 53ddade6 - Add changelog entry

Compare with previous version

changed the description

	1 Warning
	This merge request changed files with disabled eslint rules. Please consider fixing them.

Disabled eslint rules

The following files have disabled eslint rules. Please consider fixing them:

• app/assets/javascripts/clusters/components/application_row.vue

Run the following command for more details

node_modules/.bin/eslint --report-unused-disable-directives --no-inline-config \
  'app/assets/javascripts/clusters/components/application_row.vue'

Generated by Danger

added 1 commit

• 3ca65aa0 - Pretty print changed files

Compare with previous version

added 43 commits

• 3ca65aa0...93a93174 - 40 commits from branch master
• 2dfbfbab - Fix cluster installation processing state
• fa8dfbc8 - Add changelog entry
• cb38a0de - Pretty print changed files

Compare with previous version

@psimyn could you please review this MR?

assigned to @psimyn

@jerasmus left a couple of questions. Overall LGTM thanks for fixing!

assigned to @jerasmus

added 1 commit

• 9fbef901 - Address review comments

Compare with previous version

@psimyn thanks for the review!

I've addressed your comments, back to you!

assigned to @psimyn

@jerasmus all looks good. Given the rest of application_row_spec.js I think it's ok to leave as-is, but leaving discussion open for a maintainer to resolve

assigned to @jerasmus

Thanks for the review @psimyn!

@fatihacet could you please give this a maintainer review?

assigned to @fatihacet

@jerasmus the checks we have and the especially ones like (x || y || z) && !t) worries me here in this MR. I know, it was there before and you just changed some of them or added a few more etc. but still, I believe it's a problem.

For instance, we have a computed property called isInstalling and we chain it with some other computed properties to get something we want. Changing something wrong in the isInstalling property would cause other checks to fail.

How can we make this better? Since you already deep dive into this part of the codebase, do you think it's further possible to improve the logic here? Please let me know what you think about it.

assigned to @jerasmus

added 1 commit

• 757a9dd5 - Address review comments

Compare with previous version

added 1 commit

• 24790d27 - Add test for the installation button

Compare with previous version

assigned to @fatihacet

resolved all discussions

approved this merge request

merged

mentioned in commit ad47b123

LGTM Locally works fine too. Thanks @jerasmus

Woo! Thanks @jerasmus!

mentioned in commit epenance/gitlab-ce@e5bddc89

This MR introduced failures in master, and were even present in this MR but were merged anyway.

Edit: I was mistaken about this being broken in this MR. I was looking at the wrong widget. Apologies for that accusation!

mentioned in commit 1414cfae

@rspeicher oops I didn't see that. Looking into it now.

mentioned in issue #56398 (closed)

Reverted to resolve ~"broken master".

Nope, I was right the first time. It did fail in this branch, we just didn't see it because we have several builds with only: merge_requests, which were green, and hid the broken pipeline below it.

Ah, a terrible gotcha! Note rebasing with master will take away the only: merge_requests pipeline from running, probably too late to be of benefit now.

@jerasmus looks like the failures are legit, shame that we missed it. Can you rebase from master and fix failing specs then create another MR and assign to me?

My apologies, not sure how it slipped through. Tests fixed now in: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/24814

mentioned in merge request !24814 (merged)

mentioned in issue gitlab-org/release/tasks#669 (closed)

added devopsconfigure [DEPRECATED] label

added groupconfigure [DEPRECATED] label

changed the description

added groupenvironments label and removed groupconfigure [DEPRECATED] label