Add useful local tasks tabs when viewing a node revision

Created on 11 June 2017, almost 8 years ago
Updated 6 June 2023, almost 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 2 days 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 over 1 year ago
  • last update over 1 year ago
    29,953 pass
  • 🇺🇸United States DamienMcKenna NH, USA

    Rerolled for 9.5.x.

  • Status changed to Needs work over 1 year 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 about 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 about 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.
  • Pipeline finished with Success
    6 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
    6 months ago
    Total: 358s
    #294936
  • Pipeline finished with Failed
    6 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
    5 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
    5 months ago
    Total: 522s
    #301336
  • Pipeline finished with Failed
    5 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
    5 months ago
    Total: 775s
    #301346
  • Pipeline finished with Failed
    5 months ago
    Total: 733s
    #301354
  • Pipeline finished with Success
    5 months ago
    Total: 986s
    #301358
  • Pipeline finished with Running
    5 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
    5 months ago
    Total: 891s
    #303430
  • Pipeline finished with Canceled
    5 months ago
    Total: 261s
    #303490
  • Pipeline finished with Success
    5 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
    5 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
    4 months ago
    Total: 68s
    #327280
  • Pipeline finished with Failed
    4 months ago
    Total: 896s
    #327281
  • Pipeline finished with Success
    4 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
    4 months ago
    Total: 182s
    #346074
  • Pipeline finished with Failed
    4 months ago
    Total: 183s
    #346106
  • Pipeline finished with Failed
    4 months ago
    Total: 140s
    #346117
  • Pipeline finished with Failed
    4 months ago
    Total: 1878s
    #346129
  • Pipeline finished with Failed
    4 months ago
    Total: 3268s
    #346180
  • Pipeline finished with Failed
    4 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
    4 months ago
    Total: 139s
    #346426
  • Pipeline finished with Canceled
    4 months ago
    Total: 85s
    #346446
  • Pipeline finished with Failed
    4 months ago
    Total: 563s
    #346451
  • Pipeline finished with Failed
    4 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
    3 months ago
    Total: 801s
    #357528
  • Pipeline finished with Success
    3 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.

Production build 0.71.5 2024