Problem/Motivation
When a log is automatically given a name using the log type's "Name pattern", it can use tokens based on the log's fields.
If these fields change and the log is re-saved, then the log's name will not reflect the changes.
Steps to reproduce
Imagine a "Harvest" log type with an "Area reference" field, and a "Name pattern" of Harvest from [area:name]
. If I create a new harvest log that references "Area A", the log name will be "Harvest from Area A". If I then change the reference to "Area B" and resave the log, the log's name is still "Harvest from Area A"
Proposed resolution
We should try to regenerate names when logs are updated.
At quick glance, this is a bit tricky, because we only generate a log's name if it is empty when it is saved. Furthermore, we don't want to overwrite a name that was manually entered by a user. When our postSave
method runs on an existing log, it will always be non-empty (because the name was either entered by the user, or auto-generated on initial insert).
So... in order to achieve this, we would need a bit of logic to determine if the log's name was auto-generated or user-entered.
This could be accomplished by running the name-generator code using the original
entity when the log is saved, and comparing it to the original log name, to see if it matches. If it does, then the log name was auto-generated, and therefore should be auto-generated again when the log is updated.
Remaining tasks
- Add logic to check if the log name was auto-generated during initial insert
- If it was auto-generated, regenerate it on entity update
Comment | File | Size | Author |
---|---|---|---|
#23 | 3176835-23.patch | 5.81 KB | m.stenta |
#22 | 3176835-22.patch | 5.84 KB | m.stenta |
#21 | 3176835-21.patch | 5.72 KB | m.stenta |
#19 | 3176835-19.patch | 5.46 KB | m.stenta |
#17 | 3176835-17.patch | 5.77 KB | m.stenta |
Comments
Comment #2
m.stentaInitial patch for testing/review.
Comment #4
m.stentaForgot to update the other tests. Let's see if this fixes them.
Comment #6
m.stentaI think I need to figure out how to mock up timestamp field form submissions. :-)
Try again...
Comment #8
m.stentaAh ha! The tests revealed a flaw in my actual patch code (as they should)!
I was assuming that
$entity->original
was available indoPostSave()
when a log is updated... but this appears to be incorrect.So my code for comparing to the old generated name is not even running. Will have to figure out how to get the original log in this context...
Comment #9
m.stentaDoh!
EntityStorageBase->doPostSave()
runsunset($entity->original);
as it's final step: https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Entity%21...We run
parent::doPostSave($entity, $update);
right before our name generating logic, so we don't have the original entity. We do this because we need it to run afterwards in the case of new logs so that we can get the Log ID for use in token replacements.Comment #10
m.stentaThis patch adds
$original = $entity->original
before the call toparent::doPostSave($entity, $update);
so that we have the original entity for comparison.Comment #11
m.stentaOops - includes patches in my patch. This one is clean. :-)
Comment #14
m.stentaI think there is a timezone issue with my tests... the expected/actual values are exactly 11 hours off. (thinking face emoji)
Comment #15
m.stentaTesting based on timestamp/timezone was a silly idea anyway. Here is an updated patch that uses the status field instead.
Comment #17
m.stentaComment #19
m.stentaComment #21
m.stentaAh I understand why that failed. My assumptions about what was happening in
testEditLog()
were wrong. I assumed$log = $this->createLogEntity(['type' => 'name_pattern']);
was creating a log with an auto-generated name, but in fact it is setting the name to$this->randomMachineName()
.This is good. It made me rethink the test I wanted to perform, and I added another test in the process. With this updated patch,
testEditLog()
will test three things:Comment #22
m.stentaWoo hoo! Tests passed! :-)
Attaching a final patch that restores one of the comments we had in there originally, regarding the fact that
parent::doPostSave
needs to run before our name generation logic so that new log ID can be used in token replacement. I think this is important to keep for future reference.Comment #23
m.stentaSpoke with @pcambra and he suggested moving the logic above
parent::doPostSave()
(while keeping the actual token replacement logic below it). This avoids the need to save/clone$entity->original
.Updated patch attached. If this passes I think we're good to merge.
Comment #25
m.stenta