feat: handle connected services in the admin panel #3329

lorenzo-cavazzi · 2024-09-25T15:12:43Z

Handle connected services in the admin panel.

/deploy

RenkuBot · 2024-09-25T15:13:36Z

You can access the deployment of this PR at https://renku-ci-ui-3329.dev.renku.ch

leafty

Some comments about the API.

client/src/features/connectedServices/connectedServices.api.ts

client/src/features/admin/AddConnectedServiceButton.tsx

* Finish the component to add a new service. * Polish the card to show existing services. * Add the delete button.

lorenzo-cavazzi · 2024-09-27T13:14:48Z

@leafty Thank you for the early review. I incorporated your suggestions and finished the PR.

P.S. Sorry for rebasing; I changed the target branch to main and the history was 😵‍💫

client/src/features/admin/AddConnectedServiceButton.tsx

client/src/features/connectedServices/ConnectedServicesPage.tsx

client/src/features/admin/ConnectedServicesSection.tsx

leafty · 2024-09-30T12:49:00Z

client/src/features/admin/UpdateConnectedServiceButton.tsx

The update form does not work for the client secret. If the value is not changed, then do not send the field to the API. Also, do not populate the value with redacted.

Note that this is a blocker for the PR. We cannot have admins break connected services by just updating the display name for example.

Fixed in 1aef2c3

* Add provider button text * Connected serivces page button color * italics text

lorenzo-cavazzi · 2024-10-02T14:24:31Z

@leafty Thank you for the review. I applied your suggestions and fixed the problem with the client secret

leafty

Code looks good, there is one change to do, see below:

leafty · 2024-10-03T07:37:56Z

client/src/features/admin/UpdateConnectedServiceButton.tsx

+  useEffect(() => {
+    if (!result.isSuccess) {
+      return;
+    }
+    toggle();
+  }, [result.isSuccess, toggle]);


You should also reset the form here. Otherwise the client secret field is still populated when opening the form again.

That shouldn't be necessary. After adding a new service, this invokes toggle() so that the other useEffect will trigger and invoke result.reset(). I tried a few combinations to test this.

I can still add a reset here to be safe if you think there are cases that I might be missing.

Yes, add a form reset() in the useEffect() just below then 😄 . The result.reset() is for RTK Query. You also need to reset the form at the same time when you do a result.reset() in almost all cases.

Oh sorry, I wrote result.reset() but I meant reset() from the other useEffect. When you open the new modal, the previous client_secret isn't there.

Anyway, adding a reset() here still makes the code more readable (and it makes sense to reset immediately after the successful action) so I added it in d640d83.

leafty · 2024-10-03T08:52:21Z

client/src/features/admin/ConnectedServiceFormContent.tsx

+          control={control}
+          name="client_secret"
+          render={({ field }) => (
+            <Input


Also, this field could be of type password with the button to toggle it to text like we have for some other hidden values. You can push that to a new issue if you like.

That makes sense; I was undecided whether to use text or password here, but since you also mentioned this I'll switch to password -- no need for another PR 🙂

Done in ae36898

lorenzo-cavazzi · 2024-10-03T09:16:51Z

@leafty I implemented your second batch of suggestions; thanks for taking the time to go through the PR again. That helps in reducing the follow-up fixes/improvements :D

I noticed that IDs with spaces are accepted, but editing/deleting those services triggers errors. Any idea what could be wrong?
P.S: I added a disabled input to show the ID in the "update" modal.

leafty · 2024-10-03T09:26:36Z

I noticed that IDs with spaces are accepted, but editing/deleting those services triggers errors. Any idea what could be wrong? P.S: I added a disabled input to show the ID in the "update" modal.

This is a bug with the backend, I will have to fix it.

leafty

LGTM Thanks

RenkuBot · 2024-10-03T09:38:54Z

Tearing down the temporary RenkuLab deplyoment for this PR.

lorenzo-cavazzi temporarily deployed to renku-ci-ui-3329 September 25, 2024 15:13 — with GitHub Actions Inactive

leafty reviewed Sep 26, 2024

View reviewed changes

lorenzo-cavazzi temporarily deployed to renku-ci-ui-3329 September 27, 2024 09:48 — with GitHub Actions Inactive

lorenzo-cavazzi temporarily deployed to renku-ci-ui-3329 September 27, 2024 12:11 — with GitHub Actions Inactive

lorenzo-cavazzi had a problem deploying to renku-ci-ui-3329 September 27, 2024 13:04 — with GitHub Actions Error

lorenzo-cavazzi marked this pull request as ready for review September 27, 2024 13:05

lorenzo-cavazzi requested a review from a team as a code owner September 27, 2024 13:05

lorenzo-cavazzi added 6 commits September 27, 2024 15:05

refactor: change buttons color

ef6d8d3

feat: handle connected services in the admin panel (part 1)

178518e

feat: continue the Connected service admin section

ea13154

* Finish the component to add a new service. * Polish the card to show existing services. * Add the delete button.

refactor: use consistent style and spacing in the admin panel sections

0e4a418

fix typo

f0af031

feat: add update button and finalize the feature

13bfef7

lorenzo-cavazzi force-pushed the lorenzo/git-auth-flow-admin branch from 7e530cd to 13bfef7 Compare September 27, 2024 13:06

lorenzo-cavazzi changed the base branch from lorenzo/git-auth-flow to main September 27, 2024 13:06

lorenzo-cavazzi had a problem deploying to renku-ci-ui-3329 September 27, 2024 13:06 — with GitHub Actions Error

lorenzo-cavazzi temporarily deployed to renku-ci-ui-3329 September 27, 2024 13:10 — with GitHub Actions Inactive

lorenzo-cavazzi assigned leafty Sep 27, 2024

lorenzo-cavazzi temporarily deployed to renku-ci-ui-3329 September 27, 2024 13:20 — with GitHub Actions Inactive

leafty requested changes Sep 30, 2024

View reviewed changes

fixes from review (part 1)

ae60afd

* Add provider button text * Connected serivces page button color * italics text

lorenzo-cavazzi temporarily deployed to renku-ci-ui-3329 October 2, 2024 13:52 — with GitHub Actions Inactive

lorenzo-cavazzi added 2 commits October 2, 2024 15:58

refactor: use the same style already used in the nearby components

c901003

fix: prevent updating the client secret when it's redacted

1aef2c3

lorenzo-cavazzi had a problem deploying to renku-ci-ui-3329 October 2, 2024 14:24 — with GitHub Actions Failure

lorenzo-cavazzi requested a review from leafty October 2, 2024 14:24

lorenzo-cavazzi had a problem deploying to renku-ci-ui-3329 October 3, 2024 07:07 — with GitHub Actions Failure

fix

54ea02f

lorenzo-cavazzi temporarily deployed to renku-ci-ui-3329 October 3, 2024 07:16 — with GitHub Actions Inactive

leafty requested changes Oct 3, 2024

View reviewed changes

leafty reviewed Oct 3, 2024

View reviewed changes

lorenzo-cavazzi added 3 commits October 3, 2024 10:58

add additional reset

d640d83

refactor: use password for client secret

ae36898

show provider id in the update modal

78795df

lorenzo-cavazzi deployed to renku-ci-ui-3329 October 3, 2024 09:13 — with GitHub Actions View deployment

leafty approved these changes Oct 3, 2024

View reviewed changes

lorenzo-cavazzi merged commit 910491f into main Oct 3, 2024
18 checks passed

lorenzo-cavazzi deleted the lorenzo/git-auth-flow-admin branch October 3, 2024 09:38

leafty mentioned this pull request Oct 3, 2024

In connected services, handle spaces in the id field SwissDataScienceCenter/renku-data-services#437

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: handle connected services in the admin panel #3329

feat: handle connected services in the admin panel #3329

lorenzo-cavazzi commented Sep 25, 2024 •

edited

Loading

RenkuBot commented Sep 25, 2024

leafty left a comment

lorenzo-cavazzi commented Sep 27, 2024

leafty Sep 30, 2024 •

edited

Loading

lorenzo-cavazzi Oct 2, 2024

lorenzo-cavazzi commented Oct 2, 2024

leafty left a comment

leafty Oct 3, 2024

lorenzo-cavazzi Oct 3, 2024

leafty Oct 3, 2024

lorenzo-cavazzi Oct 3, 2024

leafty Oct 3, 2024

lorenzo-cavazzi Oct 3, 2024

lorenzo-cavazzi Oct 3, 2024

lorenzo-cavazzi commented Oct 3, 2024 •

edited by leafty

Loading

leafty commented Oct 3, 2024

leafty left a comment

RenkuBot commented Oct 3, 2024

feat: handle connected services in the admin panel #3329

feat: handle connected services in the admin panel #3329

Conversation

lorenzo-cavazzi commented Sep 25, 2024 • edited Loading

RenkuBot commented Sep 25, 2024

leafty left a comment

Choose a reason for hiding this comment

lorenzo-cavazzi commented Sep 27, 2024

leafty Sep 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lorenzo-cavazzi commented Oct 2, 2024

leafty left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lorenzo-cavazzi commented Oct 3, 2024 • edited by leafty Loading

leafty commented Oct 3, 2024

leafty left a comment

Choose a reason for hiding this comment

RenkuBot commented Oct 3, 2024

lorenzo-cavazzi commented Sep 25, 2024 •

edited

Loading

leafty Sep 30, 2024 •

edited

Loading

lorenzo-cavazzi commented Oct 3, 2024 •

edited by leafty

Loading