Problem/Motivation
Terms don't have changed time tracking. See various reasons why this is a problem at #2074191: [META] Add changed timestamp tracking to content entities.
Proposed resolution
Add change time tracking to terms.
Remaining tasks
Needs upgrade path.
User interface changes
None.
API changes
Schema changed. New getChangedTime method added. This naming is already used on other entity types. See #2044583: Add EntityChangedInterface to allow entities with "changed" field to be properly cached for unification of this.
Related Issues
#2044583: Add EntityChangedInterface to allow entities with "changed" field to be properly cached
#2074191: [META] Add changed timestamp tracking to content entities
Comment | File | Size | Author |
---|---|---|---|
#21 | 2074229-21-term-changed.patch | 5.99 KB | Gábor Hojtsy |
#21 | interdiff.txt | 818 bytes | Gábor Hojtsy |
#19 | interdiff.txt | 2.2 KB | Gábor Hojtsy |
#19 | 2074229-19-term-changed.patch | 5.88 KB | Gábor Hojtsy |
#16 | 2074229-16-term-changed_no-update.patch | 3.58 KB | tstoeckler |
Comments
Comment #1
Gábor HojtsyAlso needs upgrade path.
Comment #3
Gábor HojtsyUpdated patch as per findings in #2074193: Add changed time tracking to custom blocks. Should not define property on Term anymore but should define it is baseFieldDefinitions() array instead. Still needs upgrade path.
Comment #4
Wim LeersFor the upgrade path, what about this:
variable_set('d8_update_time', REQUEST_TIME)
when starting the D7 -> D8 upgrade process, then use that time for all entities that need a "changed" timestampI don't think we are able to generate better timestamps?
Comment #5
Gábor HojtsyYeah, terms don't have any kind of timestamp... We can take the request time in update. It is pretty easy to update all entities at once with a query. Or are you advocating doing entity saves for each? That, especially given that we are changing almost all entities on your site including users and stuff with all these patches sounds like would take a time that is totally prohibitive. The update will just run forever.
Comment #6
Wim LeersYes, update all entities that are of entity types that don't have a "created" timestamp in one query. Saving each entity individually would be very painful.
Comment #7
Gábor HojtsyWe can also do an update with an expression for those which have a created time to just copy that to the changed field :)
Comment #9
Gábor HojtsyNow only the upgrade path fails remained, so here is the upgrade path :)
Comment #10
tstoecklerEven though it's technically out of scope, can we add a 'created' timestamp as well, while we're at it. Having both seems to be the (emerging) standard, and since we have to provide an upgrade path etc. anyway, I think that would make sense.
Comment #11
Gábor HojtsyYeah, well, uid would also be good, and uid/created/changed combined is useful for accountability, but I'd rather not go and expand the scope :( See lack of agreement in #1295148: Maintain common properties/statistics about entities.
Comment #12
Gábor HojtsyComment #13
tstoecklerYeah, OK, I guess.
The patch itself looks but needs upgrade path tests.
Comment #14
Gábor Hojtsy@tstoeckler: ok, the prior patch failed in the upgrade test because it could not find the proper column. So if the patch with the upgrade path passes, it proves there was "some" coverage in the upgrade test :) What kind of coverage you are looking at. We cannot really tell which timestamp is the right one after update anymore, so just take a look if a != 0 timestamp is there?
Comment #15
Gábor HojtsyTotally did not want to remove the tag.
Comment #16
tstoecklerYeah, something like that sounds good. I think that is more explicit than the fails in the previous patch, that don't relate to taxonomy module at all.
Comment #17
Gábor HojtsyLooks good to me :)
Comment #18
skyredwangWhat is the chance of porting this patch to D7?
Comment #19
Gábor Hojtsy@skyredwang points in IRC to a very sensible missing piece. Views integration... IMHO we cannot just skip this.
Comment #20
skyredwangI tested the new views support in #19, fields, sort, filter, argument all work.
Comment #21
Gábor HojtsyAfter #2044583: Add EntityChangedInterface to allow entities with "changed" field to be properly cached landed, this should now use the interface proper. Should not change RTBC status, this is minimal and trivial :)
Comment #22
Wim LeersYay for #21!
Comment #23
webchickGood stuff, thanks!
Committed and pushed to 8.x.
Comment #24
skyredwangI am interested in porting #21 for D7, I was wondering if I should do it in a contributed module or directly patch against D7 core?
Comment #25
Wim LeersI very much doubt such a big change would be acceptable for D7 core. You'd have to do it in contrib.
Comment #27
skyredwangTaxonomy Changed Project is created for D7