Contrib.social

Check term view access before trying to redirect after an update

Open on Drupal.org →
Created on 29 August 2024, 4 months ago

Problem/Motivation

When a user is editing a term, then saves the form, Drupal will send the user to the term view form.

However Drupal doesn't know if the user has access to view the form. There may be custom code, such as entityaccess hooks (in my case), that change behaviour. If custom behaviour disallows view access, the user will see a access denied page.

Steps to reproduce

Proposed resolution

Check whether the user has access to view a term before trying to redirect them.

If the user does not have access, do not add a redirect. This in effect simply refreshes the update form, which we know the user has access to.

Node has had similar behaviour for over 10 years now ( #1429442: Access denied page shown after submitting form that creates a unpublished node → ), wherin it checks if a user can view a node, before trying to redirect them.

Remaining tasks

Add tests.

User interface changes

None.

Introduced terminology

None.

API changes

None.

Data model changes

None.

Release notes snippet

Minor change only.

✨ Feature request
Status

Active

Version

11.0 🔥

Component
Taxonomy  →

Last updated 2 days ago

  • Maintained by
  • 🇺🇸United States @xjm
  • 🇬🇧United Kingdom @catch
Created by

🇦🇺Australia dpi Perth, Australia

Live updates comments and jobs are added and updated live.
  • 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

  • Issue created by @dpi
  • Merge request !9365Check term view access before trying to redirect after an update → (Closed) created by dpi
  • Pipeline finished with Success
    4 months ago
    Total: 876s
    #267712
  • Pipeline finished with Failed
    4 months ago
    Total: 530s
    #267733
  • Pipeline finished with Failed
    4 months ago
    #267758
  • Issue was unassigned.
  • Status changed to Needs review 4 months ago
  • Comment 4 months ago →
    🇦🇺Australia dpi Perth, Australia
  • Pipeline finished with Failed
    4 months ago
    Total: 8938s
    #267762
  • Comment 4 months ago →
    🇦🇺Australia dpi Perth, Australia

    Test fail seems unrelated...

  • First commit to issue fork.
  • Comment 4 months ago →
    🇦🇺Australia acbramley

    One failure was related, the other didn't seem it.

  • Pipeline finished with Failed
    4 months ago
    Total: 673s
    #268695
  • Pipeline finished with Failed
    4 months ago
    Total: 666s
    #268799
  • Pipeline finished with Failed
    4 months ago
    Total: 589s
    #268841
  • Comment 4 months ago →
    🇦🇺Australia dpi Perth, Australia

    Just the unrelated Nightwatch failure now.

  • Status changed to RTBC 4 months ago
  • Comment 4 months ago →
    🇺🇸United States smustgrave

    Ran test-only feature and test coverage appears to be there.

    1) Drupal\Tests\taxonomy\Functional\TermTest::testRedirects
Behat\Mink\Exception\ExpectationException: Current page is "/taxonomy/term/3", but "/taxonomy/term/3/edit" expected.
/builds/issue/drupal-3470876/core/tests/Drupal/Tests/WebAssert.php:591
/builds/issue/drupal-3470876/core/tests/Drupal/Tests/WebAssert.php:779
/builds/issue/drupal-3470876/core/modules/taxonomy/tests/src/Functional/TermTest.php:605
FAILURES!
Tests: 10, Assertions: 253, Failures: 1.
Exiting with EXIT_CODE=1

    There was a reported nightwatch failure in HEAD and believe that's it.

    Issue summary appears complete and looking at the code review don't see anything off.

    LGTM

  • Pipeline finished with Failed
    4 months ago
    Total: 650s
    #271052
  • Comment 3 months ago →
    🇬🇧United Kingdom catch

    MR looks fine but I think we should have a follow-up to apply the same pattern across entity types - users, media etc. I looked for an existing issue to unify these but couldn't find it, pretty sure we have one though.

  • Comment 2 months ago →
    🇬🇧United Kingdom catch

    Found it, opened by me 16 years ago... #239715: UMN Usability: Where do forms redirect to? → .

  • Comment 2 months ago →
    🇬🇧United Kingdom catch

    Committed/pushed to 11.x, thanks!

  • Comment 2 months ago →
    System Message
  • Comment 2 months ago →
    System Message
    • catch → committed 3141b421 on 11.x 
      Issue #3470876 by dpi, acbramley: Check term view access before trying...
  • Status changed to Fixed about 2 months ago
  • Comment about 2 months ago →
    System Message

    Automatically closed - issue fixed for 2 weeks with no activity.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024