Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ENG-2224] "Edit Scopes" creates a new token #2029

Conversation

chth0n1x
Copy link
Contributor

Purpose

The purpose of these changes is to have the access token value page close and redirect the user to the token list view upon clicking the 'Close' button.

Summary of Changes

  • The 'Edit scopes' button was updated to 'Close'
  • The template file was updated to point to the correct translation string
  • The refresh method was updated to transition the user to the token list view

Screenshot(s)

Pasted Graphic 28

Side Effects

There should be no side effects.

QA Notes

  • Does the token value page close properly following clicking the 'Close' button
  • Is the user redirected to the token list view?

@coveralls
Copy link

Pull Request Test Coverage Report for Build 6496893388

  • 0 of 1 (0.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 71.232%

Changes Missing Coverage Covered Lines Changed/Added Lines %
app/settings/tokens/edit/controller.ts 0 1 0.0%
Totals Coverage Status
Change from base Build 6483532224: 0.0%
Covered Lines: 5943
Relevant Lines: 8129

💛 - Coveralls

@chth0n1x chth0n1x changed the title Updated refresh to close token value view after completion. [ENG-2224] "Edit Scopes" creates a new token Oct 12, 2023
Copy link
Contributor

@futa-ikeda futa-ikeda left a comment

Choose a reason for hiding this comment

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

This avoids the problem, and I think it's a possible solution, but I think a proper fix to allow users to edit the token would be preferred. I left a comment in the JIRA ticket explaining why. If you could have a crack at making the "Edit scopes" button work, that would be my preferred fix.

Copy link
Contributor

@brianjgeiger brianjgeiger left a comment

Choose a reason for hiding this comment

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

While this is a potential workaround that could work, I think there's no reason to change the original workflow. So please continue working on being able to edit the scopes from after the token creation phase, or if that just won't work anymore for some reason, explain why it's infeasible. I can't think of a reason off the top of my head why the original workflow shouldn't be possible to achieve, but I'm not in the middle of the work so you may have found something I'm unaware of.

app/settings/tokens/edit/controller.ts Show resolved Hide resolved
@chth0n1x
Copy link
Contributor Author

This avoids the problem, and I think it's a possible solution, but I think a proper fix to allow users to edit the token would be preferred. I left a comment in the JIRA ticket explaining why. If you could have a crack at making the "Edit scopes" button work, that would be my preferred fix.

@futa-ikeda this is the solution mentioned in the ticket, the decision made at scrum, and followed-up on. I had provided a solution here #2022 as well. This was the ticket for the solution you suggested. Let's meet to discuss it further.

@chth0n1x
Copy link
Contributor Author

While this is a potential workaround that could work, I think there's no reason to change the original workflow. So please continue working on being able to edit the scopes from after the token creation phase, or if that just won't work anymore for some reason, explain why it's infeasible. I can't think of a reason off the top of my head why the original workflow shouldn't be possible to achieve, but I'm not in the middle of the work so you may have found something I'm unaware of.

Agreed, this solution was provided for the team if needed.

@chth0n1x chth0n1x closed this Oct 13, 2023
@chth0n1x chth0n1x deleted the access-token-go-to-list branch October 20, 2023 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants