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

Direct people to refresh OIDC connection when needed #13126

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

mkllnk
Copy link
Member

@mkllnk mkllnk commented Feb 6, 2025

ℹ️ Funded Feature. Please track ALL ASSOCIATED WORK under the associated tracking code #11678 DFC Orders

What? Why?

Screenshot from 2025-02-07 11-38-49

@dacook made a really good start on this issue. I tailored the error message to this specific case and wrote a spec covering this.

What should we test?

  • Connect to your OIDC account.
  • Go to product import.
  • In a Rails console, inter your user id in this statement: OidcAccount.where(user_id: 0).update_all(refresh_token: "aaa", updated_at: 1.year.ago)
  • Try to import a catalog like https://env-0105831.jcloud-ver-jpe.ik-server.com/api/dfc/Enterprises/test-hodmedod/SuppliedProducts

Release notes

Changelog Category (reviewers may add a label for the release notes):

  • User facing changes
  • API changes (V0, V1, DFC or Webhook)
  • Technical changes only
  • Feature toggled

The title of the pull request will be included in the release notes.

Dependencies

Documentation updates

Review comment: The last commit adds a new style for links. Before it was looking like this:
Screenshot from 2025-02-06 17-10-51

Not the best UX but the easiest next step to implement. Next we should:

* Include link in error message instead of redirecting straight there.
  Otherwise users may feel disoriented.
* Provide a custom error message?
The colour of the link is really bad though.
@mkllnk mkllnk added the user facing changes Thes pull requests affect the user experience label Feb 6, 2025
@mkllnk mkllnk self-assigned this Feb 6, 2025
@mkllnk mkllnk marked this pull request as ready for review February 6, 2025 06:25
@mkllnk mkllnk requested a review from dacook February 6, 2025 06:25
The default aqua link colour looked aweful on orange/red background. I
tried a few different standard colours but couldn't get it right. So I
reverted to the web standard link style: underline. That's looks pretty
good. I personally think that we should use the more for links.
@mkllnk mkllnk mentioned this pull request Feb 7, 2025
4 tasks
Copy link
Member

@dacook dacook left a comment

Choose a reason for hiding this comment

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

Great work, thanks for picking this up when I ran out of steam.
I agree with the style update too 👍

Copy link
Collaborator

@rioug rioug left a comment

Choose a reason for hiding this comment

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

Nice one !

app/controllers/admin/dfc_product_imports_controller.rb Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
user facing changes Thes pull requests affect the user experience
Projects
Status: Test Ready 🧪
Development

Successfully merging this pull request may close these issues.

[DFC Orders] Improve OIDC Error Handling
3 participants