Skip to content
New issue

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

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

Already on GitHub? # 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
@FavitoX
Copy link

FavitoX commented Jan 9, 2025

Hi, is this planned for the next release? And do you have a date for the next release?

@sschu
Copy link
Contributor

sschu commented Jan 9, 2025

Definitely January, rather sooner than later.

angeloxx pushed a commit to angeloxx/terraform-provider-keycloak that referenced this pull request Jan 26, 2025
* feat(authFlow): added optional priority

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

Closes: keycloak#296

Signed-off-by: Dennis Kniep <kniepdennis@gmail.com>

* feat:(authFlow): Added priority to execution datasource

Signed-off-by: Dennis Kniep <kniepdennis@gmail.com>

---------

Signed-off-by: Dennis Kniep <kniepdennis@gmail.com>
Signed-off-by: angeloxx <angeloxx@angeloxx.it>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow for defining Flow priorities declaratively
4 participants