Add changed time tracking to menu links

We're not working on this right now.

change-tracking-menu-link.patch queued for re-testing.

Repeating the method definition in the menu link interface is not necessary.

Might make sense to wait with this until #1842858: [PP-1] Convert menu links to the new Entity Field API, also related is #2145103: Provide non-configurable field types for entity created, changed and timestamp fields, for all those issues.

#10: You're right, I should remove that (a remnant from the first patch). Fixed.

Since #1842858: [PP-1] Convert menu links to the new Entity Field API has been taking a long time already and it doesn't look like it'll be solved soon, I'd rather get this in now.
Thanks for linking to #2145103: Provide non-configurable field types for entity created, changed and timestamp fields, I did not yet know about that one. However, that too looks like something that will take quite a long time to materialize; this patch is simple and at least moves us forward :)
So AFAICT, there's no actual harm in moving forward with this patch as-is.

Yeah, I don't think it's fair to postpone this on the menu link conversion, that one is at least a couple of months away :(

And about the patch, how about retiring the 'updated' boolean which I'm pretty sure is not useful anymore (coming from D5), and rename it to changed?

#12: I'd be fine with that renaming, but unfortunately the updated DB schema field is actually being used in many places. So it seems its meaning has been changed or overloaded. It's used in:

• menu_link_delete_multiple()
• MenuLinkAccessController::checkAccess()
• MenuLinkStorageController::loadUpdatedCustomized()

So, I'd rather not include that in the scope here, because it feels out of scope and relatively dangerous to make such a change here. It also makes me wonder whether the change Gábor made in the original patch is even accurate?

-   * The menu link modification timestamp.
+   * Flag that indicates that this link was generated during the update from Drupal 5.

11: entitychangedinterface_menulink-2074203-10.patch queued for re-testing.

So, sounds liek this is only relevant for the content entities, not all menu links? The summary needs to be clarified

Patch for menu_link_content...

The patch itself looks really nice!
Some kind of test coverage would be cool as well.

I added some tests.

1. +++ b/core/modules/menu_link_content/src/Tests/LinksTest.php
   @@ -123,6 +123,27 @@ function assertMenuLinkParents($links, $expected_hierarchy) {
   +   *
   

   Let's adda a one line description.

2. +++ b/core/modules/menu_link_content/src/Tests/LinksTest.php
   @@ -123,6 +123,27 @@ function assertMenuLinkParents($links, $expected_hierarchy) {
   +    );
   +    $link->setOptions($options);
   +    $link->save();
   +    // Make sure the changed timestamp is updated.
   +    $this->assertEqual($link->getChangedTime(), REQUEST_TIME, 'Changing a menu link sets "changed" timestamp.');
   

   Mh in order to know that it is updated maybe change the REQUEST_TIME constant, save and ensure it is the new value. Here you just check that the changed item didn't changed at all.

I agree the test could be better but how can you change a constant? I tried putting a sleep() between the asserts but the changed time always equals REQUEST_TIME.

What else could we try? One idea would be to make a database query to set the changed value to zeroes, then edit the link and check the value.

Yeah that is sad. Beside of using runkit or something like that, there is no way to change the value of a define statement.
You could though test it using the REST interface or just the normal UI.

NW for #27.

No, it is not possible to change changed with the API, that happens in preSave() of the field.

The only thing you could do is to change it directly in the database, then load and re-save, so it is set to REQUEST_TIME.

However, I would just test this:

$entity->changed->value = REQUEST_TIME - 5;
$entity->save();
// assert for request time.

That should be proof enough that you are using the changed field type, which takes care of everything.

berdir, simple and beatiful!

This seems seems to cover now enough.

Committed and pushed to 8.x. Thanks!