Allow to renew expired verifications (if renewable)#8192
Allow to renew expired verifications (if renewable)#8192andreslucena merged 4 commits intodecidim:developfrom
Conversation
|
Can you review the failing tests in CI @microstudi 🙏🏽 ? Thanks |
|
Also, I think we should also a spec to check for regressions. |
andreslucena
left a comment
There was a problem hiding this comment.
As mentioned in the PR:
- there's a failing spec
- we should add a spec to catch regressions
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. @carolromero & @andreslucena feel free to chime in. |
|
I have removed the |
|
@ahukkanen @andreslucena this should be ready now. The failing spec was testing a wrong user journey (which passed the test before because the view was also wrong). Sorry for taking so long to correct this. |
There was a problem hiding this comment.
Great!
I've tested this locally and it fixes the explained problem. I also remember I have faced the same issue myself in the past.
As a side note (not related to this PR):
I'm not quite sure what the renewable attribute does other than shows this modal before the renewal, since I could also re-authorize the another_dummy_authorization_handler after it had expired and renewable was set to false. The only difference was that it skipped this modal. But then again, this is a direct verification and not a 2-step.
I also understand this is something we can control from the authorization itself, so I believe this is intended behavior specific to the another_dummy_authorization_handler.
I also noticed that in the verifications module README.md it says currently the following:
Renewable authorizations. By default a participant can't renew its authorization
Then again we default the renewable attribute to true:
But these are not related to this PR in particular. Just something I noticed when going through it.
|
You mean that README is wrong? or it should be the desired behavior and we should change the default? Not sure what this consequences can bring ... |
Yes, this is what I meant. |
* allow to renew expired verifications (if renewable) * fix test * rubocop
* allow to renew expired verifications (if renewable) * fix test * rubocop
* allow to renew expired verifications (if renewable) * fix test * rubocop
* allow to renew expired verifications (if renewable) * fix test * rubocop
🎩 What? Why?
Currently, if a 2-steps (a verification with a custom engine) is
renewableand has expired, the user is redirected to the action "new verification" which fails due the permission required is "to create" not "to renew".This PR puts the renew path first regardless the expiration status if the verification is renewable.