-
Notifications
You must be signed in to change notification settings - Fork 8
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
HP-2530 | Include more information for profile's login methods #532
Conversation
Refs: HP-2530
Refs: HP-2530
When using pre-commit for development it's nice to have ruff change the files instead of just reporting. Refs: HP-2530
Refs: HP-2530
c61bf30
to
7cb7420
Compare
HELSINKI-PROFILE-API branch is deployed to platta: https://helsinki-profile-pr532.api.dev.hel.ninja 🚀🚀🚀 |
Fixes: - WARN: FromAsCasing: 'as' and 'FROM' keywords' casing do not match Refs: HP-2530
bc70c71
to
60defd4
Compare
HELSINKI-PROFILE-API branch is deployed to platta: https://helsinki-profile-pr532.api.dev.hel.ninja 🚀🚀🚀 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, left a couple suggestions 👍
} | ||
} | ||
""" | ||
executed = user_gql_client.execute( | ||
query, auth_token_payload={"amr": amr_claim_value}, service=service | ||
) | ||
assert "errors" not in executed | ||
assert set(executed["data"]["myProfile"]["loginMethods"]) == { | ||
assert executed["data"]["myProfile"]["loginMethods"] == [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can the order change here or is it always the same? And if it can change, does the order matter? The reason I originally used set
here instead of a list
was to ignore the order of the items and just compare the values (because ["foo", "bar"] != ["bar", "foo"]
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The order should be stable at least in the tests since database doesn't affect the order. Order doesn't matter, so in that sense it could be a set
. Though a set
could potentially hide a value existing multiple times in the data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the potential duplicates were also a reason why I used sets quite a lot, as you probably noticed. My reasoning back then was that the duplicates wouldn't probably really matter, so I used sets for deduplication.
This extends the information already available from loginMethods field by adding ceatedAt and userLabel fields to the data. At the same time loginMethods field is deprecated since this new fields is meant to replace it. Refs: HP-2530
60defd4
to
2679476
Compare
|
HELSINKI-PROFILE-API branch is deployed to platta: https://helsinki-profile-pr532.api.dev.hel.ninja 🚀🚀🚀 |
This PR adds additional information fields related to a Profile's login methods (authentication methods in keycloak). Since
ProfileNode.loginMethods
is already in use, a newProfileNode.availableLoginMethods
field is added and the former is deprecated.