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

feat(authFlow): added optional priority #1040

Merged

Conversation

denniskniep
Copy link
Contributor

  • Added optional priority to AuthenticationExecution
  • Added optional priority to AuthenticationSubFLow

Closes: #296

@denniskniep denniskniep force-pushed the feature/authentication-execution-priority branch 2 times, most recently from b456625 to cb1d4e2 Compare December 22, 2024 21:57
@gim-
Copy link

gim- commented Dec 25, 2024

Have you seen #970 ?

@denniskniep
Copy link
Contributor Author

I am currently working on migrating all features that I built in my repo, while the project was not responsive.
When I finished the migration for the feature "priority field" I recognized your PR.
But I decieded to keep mine, because at the time when I was looking at your PR there were no tests included yet (see here)
Furthermore I think we should follow the pattern, that version checks should be done in code of provider package.

TBH: I don´t mind which PR we are going to merge ;)

@sschu
Copy link
Contributor

sschu commented Dec 31, 2024

@denniskniep @gim- Thanks for your work so far! I agree with @denniskniep to keep version checks in the provider package. On the other hand @gim- also adapted the data source which is missing in this PR. Can we somehow merge the two PRs?

* Added optional priority to AuthenticationExecution
* Added optional priority to AuthenticationSubFLow

Closes: keycloak#296

Signed-off-by: Dennis Kniep <kniepdennis@gmail.com>
Signed-off-by: Dennis Kniep <kniepdennis@gmail.com>
@denniskniep denniskniep force-pushed the feature/authentication-execution-priority branch from cb1d4e2 to d17baa5 Compare December 31, 2024 13:58
@denniskniep
Copy link
Contributor Author

@sschu @gim- I added priority attribute into the execution datasource

@sschu
Copy link
Contributor

sschu commented Dec 31, 2024

@gim- Are you fine with going for this PR instead of yours?

@gim-
Copy link

gim- commented Dec 31, 2024

@gim- Are you fine with going for this PR instead of yours?

Ok, I guess.

Copy link
Contributor

@sschu sschu left a comment

Choose a reason for hiding this comment

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

Thanks everybody!

@sschu sschu merged commit 2c32f52 into keycloak:main Jan 2, 2025
10 checks passed
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.

Allow for defining Flow priorities declaratively
3 participants