DivisionByZero error when sorting webforms by "Results" as a non-admin user

Created on 4 March 2024, 11 months ago

Problem/Motivation

There seems to be some kind of a problem regarding the current getLimit having possible return value of 'FALSE' and that messes up the paging divisions when user is non-admin user. At the same time I noticed that the current paging could be moved around to inside the load method to actually add paging after the user has counted the webforms the user actually has a access to.

Steps to reproduce

  1. Create new Drupal 10 install and add Webforms 6.2.x to it
  2. Edit permissions for user role "Content editor" by adding following to that role:
    • Access the webform overview page
    • Create webforms
    • Edit own webform
  3. Create new user and add role "Content editor" for it.
  4. Log out as admin and log in as "Content editor" -user
  5. Navigate to `/admin/structure/webform` and click "Results" tab, error should pop up, and since you are non-admin it is regular Drupal error page. `The website encountered an unexpected error. Try again later.` but logging out and logging in as a admin shows the error `DivisionByZero...`
  6. Try to add some webforms to notice that the same happens.

Proposed resolution

  • Add simplified getLimit method.
  • Add paging after the entity count is correct and user has access to all webforms the system is paging.

Remaining tasks

User interface changes

API changes

Data model changes

🐛 Bug report
Status

Active

Version

6.2

Component

User interface

Created by

🇫🇮Finland konstara Helsinki

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @konstara
  • Status changed to Needs review 11 months ago
  • 🇫🇮Finland konstara Helsinki
  • Merge request !411#3425433 add reworked paging and getLimit → (Closed) created by konstara
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.1.4 + Environment: PHP 8.2 & MySQL 8
    last update 11 months ago
    536 pass
  • Pipeline finished with Success
    11 months ago
    Total: 2163s
    #110513
  • Status changed to Needs work 2 months ago
  • 🇺🇸United States jrockowitz Brooklyn, NY

    This needs to be rerolled for 6.3.x with test coverage.

  • Pipeline finished with Failed
    7 days ago
    Total: 433s
    #398832
  • Pipeline finished with Failed
    7 days ago
    Total: 229s
    #398847
  • 🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

    There is no need to close the merge request. It can be updated to point to branch 6.3.x and you can force-push the updated code to the feature branch of the merge request.

  • 🇫🇮Finland konstara Helsinki

    Answer to #6: I think I messed up the branch on previous merge request and it seemed to be rather cumbersome to fix. I tried to merge the 6.3.x to the branch that was based on 6.2.x so it had all the changes shown as they are coming in on my merge request as a new files and chagnes. Closed that and created new branch based on 6.3.x and added code there as it was before. Still haven't made tests, but they are coming in the near future and then I will open the request again.

  • 🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

    You can checkout the branch of the merge request, run git reset --hard 6.3.x, make your changes, then git push --force. That will re-use the current merge request. It will automatically create a tag pointing at the old commits so they are there to refer to. Nothing gets lost.

Production build 0.71.5 2024