DOJ-120: Adding form queue handler. #78
Conversation
| * {@inheritdoc} | ||
| */ | ||
| public function sendMessage(WebformSubmissionInterface $webform_submission, array $message) { | ||
| // @var @var QueueFactory $queue_factory |
There was a problem hiding this comment.
is @var in here twice on purpose?
| $queue_factory = \Drupal::service('queue'); | ||
|
|
||
| // @var QueueInterface $queue | ||
| $queue = $queue_factory->get('foia_queue_submissions'); |
There was a problem hiding this comment.
Maybe call this something like foia_submissions_queue or just foia_submissions. The current name sounds kind of like an action.
|
|
||
| // Log the form submission. | ||
| \Drupal::logger('foia_webform') | ||
| ->info('FOIA form submission added to queue.', ['link' => $webform_submission->toLink($this->t('View'))->toString()]); |
There was a problem hiding this comment.
Good thought to log this. Slipped my mind. Mind adding the submission ID to the log message?
| /** | ||
| * Provides base functionality for the FoiaForm Queue Workers. | ||
| */ | ||
| abstract class FoiaFormQueue extends QueueWorkerBase implements ContainerFactoryPluginInterface { |
There was a problem hiding this comment.
Maybe we should give the worker a similarly formatted name? e.g. FoiaSubmissionQueueWorker
| $queue_factory = \Drupal::service('queue'); | ||
|
|
||
| // @var QueueInterface $queue | ||
| $foia_submission_queue = $queue_factory->get('foia_form_manual_submitter'); |
There was a problem hiding this comment.
I'm not sure that a queue can be described as a submitter. We might call the worker class a submitter if its submitting the items that it's processing, but the queue itself should probably just be named to describe its contents (e.g. submission, submissions, foia_submission, foia_submissions etc.). That was something I didn't agree with on the sitepoint write up
| // @var QueueInterface $queue | ||
| $foia_submission_queue = $queue_factory->get('foia_form_manual_submitter'); | ||
| $submission = new \stdClass(); | ||
| $submission->sid = $webform_submission->id();; |
| \Drupal::logger('foia_webform') | ||
| ->info('FOIA form submission #%sid added to queue.', | ||
| [ | ||
| '%sid' => $webform_submission->id(), |
| /** | ||
| * Provides base functionality for the FoiaForm Queue Workers. | ||
| */ | ||
| abstract class FoiaSubmissionQueueWorker extends QueueWorkerBase implements ContainerFactoryPluginInterface { |
There was a problem hiding this comment.
Because we're only planning on having a single queue worker (we don't plan on leveraging Drupal's built in Cron system, so we won't have a manual vs cron worker) I'm not sure there is added value in implementing this as an abstract class. I'm thinking we can just have this as our concrete worker (i.e. let's add the @QueueWorker annotation here)
| * | ||
| * @var \Drupal\webform\WebformSubmissionInterface | ||
| */ | ||
| protected $formSubmission; |
There was a problem hiding this comment.
Is this not an instance of WebformSubmissionStorageInterface? See WebformSubmissionLogController.php. Maybe this should be:
/**
* The webform submission storage.
*
* @var \Drupal\webform\WebformSubmissionStorageInterface
*/
protected $webformSubmissionStorage;
| /** | ||
| * {@inheritdoc} | ||
| */ | ||
| public function __construct(WebformSubmissionInterface $submission) { |
There was a problem hiding this comment.
Same comment here - think this should be WebformSubmissionStorageInterface $webformSubmissionStorage
| } | ||
|
|
||
| /** | ||
| * Adds the submission to the foia_form_manual_submitter queue. |
There was a problem hiding this comment.
This comment needs to be cleaned up to reference the right queue name. Or you can keep it generic and say that the submission is added to a queue - whatever your prefer!
| * title = @Translation("Manual FOIA Form submitter"), | ||
| * ) | ||
| */ | ||
| class FoiaSubmissionQueueWorker extends QueueWorker {} |
There was a problem hiding this comment.
You had this right previously - must've been a typo, but this should extend QueueWorkerBase.
Actually, this must've gotten lost in our communications. This should still have a constructor that instantiates the webform_submission storage.
|
Covered by #79, which was merged. |
…on annual_foia_report_data. (usdoj#78) * FOIA-37: Removes total_annual_data content type. * FOIA-37: Updates paragraph reference and continues to polish tabbing on annual_foia_report_data.
No description provided.