-
Notifications
You must be signed in to change notification settings - Fork 4
Improve UI/UX for Terms and Conditions Review (CFM-501) #419
base: develop
Are you sure you want to change the base?
Conversation
app/plugins/TermsAgreer/templates/AgreementCollectors/dispatch.inc
Outdated
Show resolved
Hide resolved
78f7593 to
1d5da04
Compare
|
All fixes made and PR rebased against the latest develop. |
| @@ -78,6 +78,7 @@ public function initialize(array $config): void { | |||
| $this->setAllowLookupPrimaryLink(['agree', 'proxy', 'revoke']); | |||
| $this->setAllowLookupRelatedPrimaryLink(['status' => ['person_id']]); | |||
| $this->setAllowUnkeyedPrimaryLink(['review']); | |||
| $this->setAllowEmptyPrimaryLink(['review']); | |||
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.
Sorry, I missed this before -- under what circumstances are we allowing the Primary Link to be empty? ie, why are we calling the review action without a CO?
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.
In Slack, you said "I added that originally because without it I get the runtime exception “Could not find value for Primary Link” at the point at which I attempt to save the T&Cs (e.g. at /registry-pe/terms-and-conditions/review?co_id=2"
The Primary Link (co_id) is provided, so "Allow Empty" is not right. This should be handled by setAllowUnkeyedPrimaryLink instead. However, you say "I attempt to save the T&Cs", which implies a POST of some form that is dropping the co_id from the submission. I'll need to reread your PR to see what's going on, but setAllowEmptyPrimaryLink is almost certainly not the right approach.
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.
Ah - let me take a look at the post. Maybe the co_id is getting lost.
1d5da04 to
44504dc
Compare
This PR moves the plugins/TermsAgreer interface into the core Terms and Conditions interface for CO-level Terms and Conditions Review. The plugin now uses the core interface template (using an include). The result looks like the following screenshots: