Add useful local tasks tabs when viewing a node revision

Created on 11 June 2017, about 8 years ago
Updated 6 June 2023, about 2 years ago

Node local tasks tabs do not appear on node revisions on entity.node.revision route. As a content author, I would expect to see the tabs when navigating to previous revisions, so I could navigate back to the revisions path, view or edit the node.

Steps To Reproduce

* Install Drupal Standard profile
* Create basic page content item and save
* Edit same page node, create new revision
* Go to node's revisions tab, node/1/revisions
* Click the initial node revision to view it, /node/1/revisions/1/view
* Observe no tabs present on revision path

Notes

* adding the following link to node.links.task.yml, displays all tabs, but duplicates Revisions tabs: 1 for entity.node.version_history route/task and one for entity.node.revision route/task

entity.node.revision:
  route_name: entity.node.revision
  base_route: entity.node.canonical
  title: 'Revisions'
📌 Task
Status

Needs work

Version

11.0 🔥

Component
Menu system 

Last updated about 12 hours ago

Created by

🇺🇸United States jasonawant New Orleans, USA

Live updates comments and jobs are added and updated live.
  • Usability

    Makes Drupal easier to use. Preferred over UX, D7UX, etc.

  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • 🇳🇱Netherlands yoroy

    Had a quick look at how Google docs and Gitlab wiki pages do this. Both explicitly *remove* the UI for editing when showing an older revision of a document.

    Google docs adds a primary button to revert to this revision

    Gitlab wiki only points back to the revision history overview

    So I tend to agree with @29 in that we should not add the standard set of node local actions. Adding the option to directly restore to the currently shown revision seems useful. How's this for a new title for this issue?

  • 🇦🇺Australia tvhung

    I've re-rolled the patch for Drupal 10.1

  • Status changed to Needs review almost 2 years ago
  • last update almost 2 years ago
    29,953 pass
  • 🇺🇸United States damienmckenna NH, USA

    Rerolled for 9.5.x.

  • Status changed to Needs work almost 2 years ago
  • 🇺🇸United States smustgrave

    Moving to NW for the test

    Thanks!

  • 🇳🇿New Zealand john pitcairn

    Based on the code in this patch, I've implemented a revision pager of sorts as secondary tasks:

    I don't really see a problem with keeping the node canonical tabs in place if revision-related tasks are always secondary. I'm considering adding a "Revert" option to this.

    I originally had something like this way back in Drupal 6 and remember my editors liking it. The revision label changes to "Latest revision (xx)" or "Published revision (xx)" when on those revisions.

    But maybe making navigating between revisions easier is something worth considering here? It's easy to get lost if you have to keep going back to the full revision list.

  • last update over 1 year ago
    29,672 pass
  • Status changed to Needs review over 1 year ago
  • 🇮🇳India djsagar

    Verified and tested #38 patch applied successfully and resolved.

    Steps to reproduce
    1. Install Drupal Standard profile
    2. Set Claro as the default theme.
    3. Create basic page content item and save
    4. Edit same page node, create new revision
    5. Go to node's revisions tab, node/1/revisions
    6. Click the initial node revision to view it, /node/1/revisions/1/view
    7. Observe no tabs present on revision path

    For reference:
    Before patch

    After patch

    Results:
    After applying the patch, the Node Local Tasks tabs are visible on node revisions.
    RTBC ++

  • Status changed to Needs work over 1 year ago
  • 🇺🇸United States smustgrave

    Please please read the tags and issue summary, this was tagged for tests which are still needed.

  • 🇮🇳India djsagar

    @smustgrave My apologies. I will adhere to tags moving forward.

  • 🇬🇧United Kingdom farse

    I can confirm that the patch in https://www.drupal.org/project/drupal/issues/2885278#comment-15144519 📌 Add useful local tasks tabs when viewing a node revision Needs work works for Drupal 10.1.6 version.

  • First commit to issue fork.
  • 🇬🇧United Kingdom oily Greater London
  • 🇬🇧United Kingdom oily Greater London

    I cannot apply patch #37 or #38 to the fork for Drupal 11.x. A reroll is required

  • 🇬🇧United Kingdom oily Greater London

    I have added commits to the issue fork. I have used an existing functional test function but added assertions. There is now a failing test that will hopefully pass once the the patch(es) are applied.

    I tried re-rolling both patches at #37 and #38 without success. It would be good if someone who has worked on the fix could add commits to the issue fork to see if they can make the test pass.

  • First commit to issue fork.
  • Merge request !9640Created MR for the changes in patch #37. → (Open) created by mrinalini9
  • Pipeline finished with Success
    9 months ago
    Total: 641s
    #294256
  • 🇬🇧United Kingdom oily Greater London
  • 🇬🇧United Kingdom oily Greater London

    Changing the status since the functional test assertions are passing in the pipeline following #50.

  • 🇬🇧United Kingdom oily Greater London
  • 🇬🇧United Kingdom oily Greater London

    I'm not sure why but the MR does not contain the new test assertions. They were added to the fork in an old commit. Will need to add them once again to prove that the 'fix' makes the test pass.

  • Pipeline finished with Failed
    9 months ago
    Total: 358s
    #294936
  • Pipeline finished with Failed
    9 months ago
    #294955
  • 🇬🇧United Kingdom oily Greater London

    Not sure if phpunit has been upgraded but my local phpunit has stopped working. Seems the test assertions for this issue are also not running in the pipeline. The code is added to the MR and the MR should be ready to review (if phpunit is working).

  • 🇬🇧United Kingdom oily Greater London
  • 🇬🇧United Kingdom oily Greater London
  • Pipeline finished with Failed
    9 months ago
    Total: 2432s
    #301221
  • 🇬🇧United Kingdom oily Greater London

    oily changed the visibility of the branch 2885278-add-useful-local to hidden.

  • Pipeline finished with Failed
    9 months ago
    Total: 522s
    #301336
  • Pipeline finished with Failed
    9 months ago
    Total: 249s
    #301339
  • 🇬🇧United Kingdom oily Greater London

    oily changed the visibility of the branch 2885278-add-useful-local to active.

  • 🇬🇧United Kingdom oily Greater London
  • Pipeline finished with Failed
    9 months ago
    Total: 775s
    #301346
  • Pipeline finished with Failed
    9 months ago
    Total: 733s
    #301354
  • Pipeline finished with Success
    9 months ago
    Total: 986s
    #301358
  • Pipeline finished with Running
    9 months ago
    #301367
  • 🇬🇧United Kingdom oily Greater London

    I have created a new FunctionalJavascript test using the Claro (admin) theme. It is passing with the MR applied. Changing the status to 'Needs Review'.

  • 🇬🇧United Kingdom oily Greater London
  • 🇬🇧United Kingdom oily Greater London
  • 🇬🇧United Kingdom oily Greater London
  • 🇬🇧United Kingdom oily Greater London
  • 🇬🇧United Kingdom oily Greater London
  • 🇬🇧United Kingdom oily Greater London
  • 🇬🇧United Kingdom oily Greater London
  • Pipeline finished with Failed
    9 months ago
    Total: 891s
    #303430
  • Pipeline finished with Canceled
    9 months ago
    Total: 261s
    #303490
  • Pipeline finished with Success
    9 months ago
    Total: 689s
    #303494
  • The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

    This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

    Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

  • Pipeline finished with Success
    9 months ago
    Total: 448s
    #306518
  • 🇬🇧United Kingdom oily Greater London

    Applied bot fix.

  • 🇬🇧United Kingdom oily Greater London
  • 🇸🇬Singapore anish.a Singapore
  • 🇬🇧United Kingdom oily Greater London
  • 🇺🇸United States smustgrave

    Left comment on MR.

  • Pipeline finished with Canceled
    8 months ago
    Total: 68s
    #327280
  • Pipeline finished with Failed
    8 months ago
    Total: 896s
    #327281
  • Pipeline finished with Success
    8 months ago
    Total: 1556s
    #327290
  • 🇬🇧United Kingdom oily Greater London

    Test is moved to the node module.

  • The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

    This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

    Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

  • Pipeline finished with Failed
    7 months ago
    Total: 182s
    #346074
  • Pipeline finished with Failed
    7 months ago
    Total: 183s
    #346106
  • Pipeline finished with Failed
    7 months ago
    Total: 140s
    #346117
  • Pipeline finished with Failed
    7 months ago
    Total: 1878s
    #346129
  • Pipeline finished with Failed
    7 months ago
    Total: 3268s
    #346180
  • Pipeline finished with Failed
    7 months ago
    Total: 141s
    #346352
  • Merge request !10289Resolve #2885278 "New 11.x" → (Open) created by oily
  • 🇬🇧United Kingdom oily Greater London

    oily changed the visibility of the branch 2885278-add-useful-local to hidden.

  • Pipeline finished with Failed
    7 months ago
    Total: 139s
    #346426
  • Pipeline finished with Canceled
    7 months ago
    Total: 85s
    #346446
  • Pipeline finished with Failed
    7 months ago
    Total: 563s
    #346451
  • Pipeline finished with Failed
    7 months ago
    Total: 824s
    #346461
  • 🇬🇧United Kingdom oily Greater London

    After running into rebasing problems, created a new 11.x branch and added the code changes to it.

    Several tests need to be fixed.

  • First commit to issue fork.
  • Pipeline finished with Failed
    7 months ago
    Total: 801s
    #357528
  • Pipeline finished with Success
    7 months ago
    Total: 1070s
    #357588
  • 🇮🇳India ramprassad

    I have committed the changes to fix the MR, now the failing tests are fixed. Please check

  • 🇺🇸United States smustgrave

    Left comments.

  • 🇬🇧United Kingdom oily Greater London

    The tests seem fixed now, pipeline runs all green except for the test-only test: fails like it should.

    That seems to leave several out-of-scope typehints that need to be removed from the MR.

  • 🇺🇸United States smustgrave

    May need to think approach as not seeing why we are adding comment hooks into node. Should try and keep things separate if possible.

  • First commit to issue fork.
  • 🇨🇦Canada sstapleton

    I went through the MR and removed the type hinting and the node hooks as per the MR review.

  • Pipeline finished with Failed
    about 1 month ago
    Total: 496s
    #504928
  • 🇺🇸United States smustgrave

    Hooks will need to be converted to OOP functions

  • Pipeline finished with Failed
    13 days ago
    Total: 71s
    #518806
  • 🇨🇦Canada sstapleton

    Marking as needs review since I updated the merge request to use a hook object instead of a regular hook.

  • Pipeline finished with Failed
    13 days ago
    Total: 169s
    #518809
  • Pipeline finished with Failed
    13 days ago
    Total: 605s
    #518833
  • 🇨🇦Canada sstapleton

    I wasn't able to apply the patch on 10.4.8, so I re-applied the changes here.

    https://www.drupal.org/files/issues/2025-06-10/2885278-add-local-tasks-t...

  • 🇺🇸United States smustgrave

    Left comments on MR

  • Pipeline finished with Failed
    4 days ago
    Total: 169s
    #526548
  • Pipeline finished with Failed
    4 days ago
    Total: 114s
    #526554
  • Pipeline finished with Failed
    4 days ago
    Total: 143s
    #526558
  • Pipeline finished with Failed
    4 days ago
    Total: 170s
    #526560
  • 🇨🇦Canada sstapleton

    Moved the config translation hook over to the NodeHooks file and removed the additional NodeLocalTaskHooks.
    Fixed some lint errors.

    https://git.drupalcode.org/project/drupal/-/merge_requests/10289

  • 🇺🇸United States smustgrave

    Thanks for fixing those, seems we are getting test-failures now, re-ran twice

  • 🇨🇦Canada sstapleton

    Okay, thanks for your time. I'll fix the tests. To be clear, are there specific tests that need to be passed or all of them?
    I am not sure if some tests may be failing due to something unrelated.

Production build 0.71.5 2024