"Regenerate Key" button creates a duplicate API key

[abusarah-tech]

Bug Description

When creating an API key on the UI, and then clicking the "Regenerate Key" button, I notice a new key is created under Settings > Developers.

Expected behavior

When clicking on "Regenerate Key" button, I do not want a duplication of the key but rather overwrite the same key.

[FelixMalfait]

Thanks for the report! I can reproduce it

[sohalkumar]

Hi @Abuhafsa & @FelixMalfait. I am very new to open source and was looking at this issue to contribute. I noticed that the duplicates 'go away' after a refresh of the webpage.
I am still trying to understand the codebase and how the rendering is happening, hence I am unable to offer a solution yet.

[FelixMalfait]

Thanks @sohalkumar

The old key isn't really removed from the database, we just add a date on the "revokedAt" column. Probably this is done correctly on the database side but isn't properly reflected on the Apollo cache. So when we go back to the page that lists API Keys, Apollo doesn't call the server again.

Probably you can use refetchQueries. Might be something smarter to do but I'm not sure

[sohalkumar]

Hi @FelixMalfait

So when we go back to the page that lists API Keys, Apollo doesn't call the server again.

This could also explain the reason that keys are not 'removed' from the table after we delete(disable) them. They are also only removed after a refresh.

Probably you can use refetchQueries. Might be something smarter to do but I'm not sure

I will try my best to look into it and possibly come up to a solution.

[charlesBochet]

@FelixMalfait @sohalkumar We don't have to use refetch query. It should be handled by the optimistic cache logic.

In the idea, the initial graphql query should be filtering on revokedAt: null
When we update an API key (setting revokedAt to null), the updateObject function should call the triggerUpdateRecordOptimisticEffect. This code should see that the initial graphql is filtering on revokedAt, loop over the api key returned return and see that the updated api key no longer matches the request. The check logic is in isRecordMatchingFilter function

We might have a bug there!

[sohalkumar]

Hi @charlesBochet

In the idea, the initial graphql query should be filtering on revokedAt: null When we update an API key (setting revokedAt to null), the updateObject function should call the triggerUpdateRecordOptimisticEffect.

I looked at the triggerUpdateRecordOptimisticEffect function and I think I have found the bug.

const updatedRecordShouldBeAddedToRootQueryEdges =
            updatedRecordMatchesThisRootQueryFilter &&
            updatedRecordIndexInRootQueryEdges === -1;

const updatedRecordShouldBeRemovedFromRootQueryEdges =
            updatedRecordMatchesThisRootQueryFilter &&
            updatedRecordIndexInRootQueryEdges === -1;`

Both of these have the exact same logic which leads to the 'regenerated key' being added due to the logic in updatedRecordShouldBeAddedToRootQueryEdges but the 'revoked key' not being removed due to the same logic in updatedRecordShouldBeRemovedFromRootQueryEdges

What I propose is changing the updatedRecordShouldBeRemovedFromRootQueryEdges like so -

const updatedRecordShouldBeRemovedFromRootQueryEdges =
            !updatedRecordMatchesThisRootQueryFilter &&
            updatedRecordIndexInRootQueryEdges !== -1;`

If I am going in the right direction please assign this issue to me.
Thanks.

[charlesBochet]

@sohalkumar I believe this has been fixed recently actually. Could you see if the issue still happens on main branch?

[sohalkumar]

@charlesBochet

@sohalkumar I believe this has been fixed recently actually. Could you see if the issue still happens on main branch?

Yes the issue is fixed and there is no duplicates anymore but the modal(pop up) that asks for the confirmatioin does not go away even after typing yes and clicking on re generate. If I click out of it the regenetated key is not visible.

clicked on re generate but still doesn't go away.

clicked outside the pop up and the key is gone

[FelixMalfait]

oh good catch @sohalkumar can you try fixing that one then :)) ? Thanks!

[sohalkumar]

Hi @FelixMalfait

oh good catch @sohalkumar can you try fixing that one then :)) ? Thanks!

setIsRegenerateKeyModalOpen(false);

adding this in the end of the regenerateApiKey function seems to work.

[FelixMalfait]

Would be best to manage this in ConfirmationModal to make sure it works for every new modal we create in the future :)

[sohalkumar]

Hi @FelixMalfait

Would be best to manage this in ConfirmationModal to make sure it works for every new modal we create in the future :)

What I could think of is adding the following in the onClick of "Regenerate Key" button inside ConfirmationModal -

onClick={async () => {
              await onConfirmClick();
              setIsOpen(false);
            }} 

This works but there must, obviously, be a better approach but I can't seem to figure it out as of now :')

[charlesBochet]

@sohalkumar The approach you describe seems to work! :) what's wrong with it?

[sohalkumar]

Hello @charlesBochet

I just thought there might be a better approach to solve this ^_^"
If this is alright , what are the next steps? Do I raise a PR?
Thanks :D

[charlesBochet]

Yes, please go ahead, looks to be a good solution!

[sohalkumar]

Hi @charlesBochet I just raised a PR. Please review it.
Thanks :D

[charlesBochet]

PR has been merged, thank you! Closing this issue

[sohalkumar]

@charlesBochet
Couldn't have been possible without your detailed responses and help.
Thank you.