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

Lb/current user method #40

Merged
merged 2 commits into from
Dec 19, 2023
Merged

Lb/current user method #40

merged 2 commits into from
Dec 19, 2023

Conversation

elceebee
Copy link
Contributor

@elceebee elceebee commented Dec 19, 2023

Context

See slack thread: https://ukgovernmentdfe.slack.com/archives/C06962474AC/p1702994098513929

We need the current_user method to return a Service::User object, not a DfeSignInUser instance.

I did this as part of the support_user journey work, it makes sense to implement it earlier as needed in other places.

Changes proposed in this pull request

When a user is logged in a service, the current_user method returns the appropriate user object - Placements::User for the placements service, Claims::User for the claims service

No UI changes.

Guidance to review

Link to Trello card

Things to check

  • [NA] If the code removes any existing feature flags, a data migration has also been added to delete the entry from the database
  • [NA] This code does not rely on migrations in the same Pull Request
  • [NA] If this code includes a migration adding or changing columns, it also backfills existing records for consistency
  • [NA] If this code adds a column to the DB, decide whether it needs to be in analytics yml file or analytics blocklist
  • [NA] API release notes have been updated if necessary
  • [NA] If it adds a significant user-facing change, is it documented in the CHANGELOG?
  • [NA] Required environment variables have been updated or added to the Azure KeyVault

Screenshots

@@ -4,7 +4,7 @@

feature "Sign In as Persona" do
around do |example|
Capybara.app_host = "https://#{ENV["PLACEMENTS_HOST"]}"
Capybara.app_host = "https://#{ENV["CLAIMS_HOST"]}"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is a quick fix for the persona test -- I'm actually splitting it into two different tests (one for claims, one for placements in the next PR, but I just want to get the current_user stuff in so we can all use it for now.

Comment on lines +21 to +23
it do
is_expected.to validate_uniqueness_of(:provider_code).case_insensitive
end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Linting, not sure how this snuck in...

@elceebee elceebee force-pushed the lb/current_user_method branch from 36b1d58 to 35c19c9 Compare December 19, 2023 15:04
@JamieCleare2525 JamieCleare2525 self-requested a review December 19, 2023 15:08
Copy link
Contributor

@JamieCleare2525 JamieCleare2525 left a comment

Choose a reason for hiding this comment

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

Resolves the issue of User being assigned to :current_user

@elceebee elceebee merged commit f3d22de into main Dec 19, 2023
4 checks passed
@elceebee elceebee deleted the lb/current_user_method branch December 19, 2023 17:33
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.

3 participants