Feature/digital signature modifications#20
Conversation
| if (($handler['id'] ?? NULL) !== 'digital_post_sf1601') { | ||
| continue; | ||
| } | ||
| $handlers[$handlerId]['settings']['additional']['states'] = ['completed']; |
There was a problem hiding this comment.
Why not use WebformSubmissionInterface::STATE_COMPLETED here?
jekuaitk
force-pushed
the
feature/digital-signature-modifications
branch
from
8b1e9cc to
000ec3f
Compare
| foreach ($handlers as $handlerId => $handler) { | ||
| if (($handler['id'] ?? NULL) !== 'digital_post_sf1601') { | ||
| continue; | ||
| } | ||
| $handlers[$handlerId]['settings']['additional']['states'] = [WebformSubmissionInterface::STATE_COMPLETED]; |
There was a problem hiding this comment.
This code is confusing: It's easy to get the impression that $handler['id'] and $handlerId are different names for the same value. They often are, but may not be.
Consider adding a comment on what $handler['id'] is and what $handlerId is.
There was a problem hiding this comment.
Renamed $handlerId to $handlerKey and added a comment explaining what $handler['id'] is.
| */ | ||
| public function setCompleted(WebformSubmissionInterface $webformSubmission) { | ||
| $webformSubmission->set('in_draft', FALSE); | ||
| $webformSubmission->setCompletedTime(time()); |
There was a problem hiding this comment.
We should use the request time here (cf. https://git.drupalcode.org/project/webform/-/blob/6.3.x/src/Entity/WebformSubmission.php?ref_type=heads#L883-898). We should always use the request time when we need the time.
| * Configure the webform submission as draft. | ||
| */ | ||
| public function setDraft(WebformSubmissionInterface $webform_submission) { | ||
| $webform_submission->set('in_draft', TRUE); |
There was a problem hiding this comment.
Consider (un)settings completed here to be on the safe side.
There was a problem hiding this comment.
Added $webformSubmission->setCompletedTime(0);
| /** | ||
| * Configure the webform submission as completed. | ||
| */ | ||
| public function setCompleted(WebformSubmissionInterface $webformSubmission) { |
There was a problem hiding this comment.
It may be worth noting that setDraft and setCompleted are each others opposites.
It may also be worth renaming them: setSubmissionCompleted may be a better name – but setSubmissionDraft is not …
There was a problem hiding this comment.
Renamed to setSubmissionCompleted and setSubmissionDraft although you don't like the latter. Also added comment explaining that they are each others complementary functions.
rimi-itk
left a comment
rimi-itk
left a comment
There was a problem hiding this comment.
Looks good. Two suggestions added, but nothing really important.
2 participants
https://leantime.itkdev.dk/_#/tickets/showTicket/7768