[18.0][ADD] base_tier_validation_confirm_auth #1213
Conversation
ArnauCForgeFlow
changed the title
ArnauCForgeFlow
force-pushed
the
18.0-add-base_tier_validation_password_confirm
branch
3 times, most recently
from
2cc208a to
8777a22
Compare
ArnauCForgeFlow
changed the title
ArnauCForgeFlow
force-pushed
the
18.0-add-base_tier_validation_password_confirm
branch
from
8777a22 to
41d98d1
Compare
LoisRForgeFlow
left a comment
LoisRForgeFlow
left a comment
There was a problem hiding this comment.
LGTM, functional test and code review 👍
|
@pedrobaeza @etobella What do you think? |
|
From a technical perspective it look Ok, for testing it functionally, I need to do it from a local environment, I will check it |
pedrobaeza
left a comment
pedrobaeza
left a comment
There was a problem hiding this comment.
About the name, I would follow from left to right the process flow, and short it, so I would call it base_tier_validation_confirm_auth.
LauraCForgeFlow
left a comment
LauraCForgeFlow
left a comment
There was a problem hiding this comment.
Functional + code review LGTM!
|
This PR has the |
ArnauCForgeFlow
force-pushed
the
18.0-add-base_tier_validation_password_confirm
branch
from
41d98d1 to
737f99c
Compare
ArnauCForgeFlow
changed the title
|
@pedrobaeza changed the name to base_tier_validation_confirm_auth as suggested! |
ArnauCForgeFlow
force-pushed
the
18.0-add-base_tier_validation_password_confirm
branch
from
737f99c to
7e6444f
Compare
|
OK, thanks. Waiting for Enric's confirmation to merge. |
etobella
left a comment
etobella
left a comment
There was a problem hiding this comment.
Tested with other kinds of authentication, like passkey.
Worked like a charm
/ocabot merge nobump
|
On my way to merge this fine PR! |
|
@etobella your merge command was aborted due to failed check(s), which you can inspect on this commit of 18.0-ocabot-merge-pr-1213-by-etobella-bump-nobump. After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red. |
|
@etobella the merge command failed, and the error seems not to be related with my module, could you try again, please? |
|
Can you rebase first? |
ArnauCForgeFlow
force-pushed
the
18.0-add-base_tier_validation_password_confirm
branch
from
7e6444f to
c20f734
Compare
|
@etobella done! |
|
there is something wrong there. Probably related to a translation made. Can you check it? 🙏 |
|
@etobella The error is related with a bad translation from base module, I created a pr trying to solve this. |
6 participants
Added a new module base_tier_validation_authentication_confirm that allows us setting an authentication confirmation during the tier validation process. The system will ask the user to authenticate himself in order to validate or reject the review. If the comment option is enabled, the password confirmation will be requested after the comment wizard.
Depends on -> #1217