statamic
prevent crated_at being null while submitting a from submission
Hi , i was tinkering with form submission then i noticed that created_at being null. Can you review my changes to see if it fixes the problem. thank you
Hi, beside the modifications you've made I'd suggest to make the firstOrNew attribute based on the id, not on form_id and timestamp, to prevent parallel submissions with the same timestamp to overwrite each other. `public function toModel() { $class = app('statamic.eloquent.forms.submission_model'); $timestamp = (new $class)->fromDateTime($this->date());
$model = $class::findOrNew($this->id());
return (!empty($model->id)) ? $model->fill([
'data' => $this->data,
]) : $model->fill([
'data' => $this->data,
'form_id' => $this->form->model()->id,
'created_at' => $timestamp,
]);
}`
This breaks all made submissions when they are shown in statamic and is therefore a critical bug.
@jasonvarga can you take a look and merge if possible?
The overwrites are real, I just checked a project where I use the latest version of eloquent-driver and some submissions got overwritten… Fortunately, Emails were sent for the certain submissions, so the data isn't lost.
@enespolat24 would you mind adding some tests, which cover these two bugs, so they never happen again?
@okaufmann i made some changes. I don't fully understand the overwrite situation. Do you have a chance to send an example case or video?
Thank you for your help !
@enespolat24 I have the tests working... can you add me to your fork so I can push to it?
@enespolat24 Sorry for the delay.
I don't fully understand the overwrite situation. Do you have a chance to send an example case or video? In the current state, the date field will not be saved. Therefore, a form can only have one submission (as the filter is form_id and created_at, where created_at is always null).
@ryanmitchell Is there anything I could help? I have time now
@okaufmann i added a test here - seems to work ok for me: https://github.com/statamic/eloquent-driver/blob/cc3780c3a9654346206cb1de4ba96a9c181ca234/tests/Forms/FormSubmissionTest.php#L56
@ryanmitchell I would also freeze the time, so it creates two submissions width the same timestamp. So we can be sure it does not overwrite any submission.
@okaufmann that will fail due to the unique constraint in the database schema. Two submissions should never have the same microsecond time...
@jasonvarga can you check if this is mergeable. This fixes a serious problem we face