Problem/Motivation

When you edit a Task, there is a link that says "Watch this task" but clicking it does not seem to do anything. Going to the api endpoint it is calling results in this error

The website encountered an unexpected error. Try again later.

Error: Using $this when not in object context in Drupal\burndown\Controller\TaskController::addToWatchlist() (line 293 of modules/contrib/burndown/src/Controller/TaskController.php).
call_user_func_array() (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->{closure:Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber::wrapControllerExecutionInRenderContext():121}() (Line: 634)
Drupal\Core\Render\Renderer::{closure:Drupal\Core\Render\Renderer::executeInRenderContext():634}()
Fiber->resume() (Line: 649)
Drupal\Core\Render\Renderer->executeInRenderContext() (Line: 121)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext() (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->{closure:Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber::onController():96}() (Line: 183)
Symfony\Component\HttpKernel\HttpKernel->handleRaw() (Line: 76)
Symfony\Component\HttpKernel\HttpKernel->handle() (Line: 53)
Drupal\Core\StackMiddleware\Session->handle() (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle() (Line: 28)
Drupal\Core\StackMiddleware\ContentLength->handle() (Line: 118)
Drupal\page_cache\StackMiddleware\PageCache->pass() (Line: 92)
Drupal\page_cache\StackMiddleware\PageCache->handle() (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle() (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle() (Line: 53)
Drupal\Core\StackMiddleware\AjaxPageState->handle() (Line: 54)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle() (Line: 745)
Drupal\Core\DrupalKernel->handle() (Line: 19)

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork burndown-3610985

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

swirt created an issue. See original summary.

jeremylichtman’s picture

jeremylichtman commented

The offending code (also pertains to https://www.drupal.org/project/burndown/issues/3610984) is in TaskController around line 286:

 // Load the user.
    $user = User::load($user_id);
    if ($user === FALSE) {
      throw new NotFoundHttpException();
    }

    // Check if this is the current user.
    <strong>$current_user = $this->account;</strong>
    if ($current_user->id() !== $user_id) {
     <strong> // Only allow admins to subscribe somebody else.</strong>
      $current_user_roles = $current_user->getRoles();
      if (!in_array('administrator', $current_user_roles)) {
        throw new NotFoundHttpException();
      }
    }

Firstly, I have no idea why $this is present. We need to load the current user properly, and test whether it is the same user as the id being added to the watchlist.

Secondly, we probably need to add a role for whether somebody can add a different user to the watchlist.

Do you want to address, or should I?

jeremylichtman’s picture

jeremylichtman commented
Assigned: Unassigned » jeremylichtman
jeremylichtman’s picture

jeremylichtman commented

Added a MR with fixes for the add/remove watchlist endpoints, as well as adding two permissions for those actions (i.e. instead of checking for admin).

https://git.drupalcode.org/project/burndown/-/merge_requests/41

Not sure if that's the only thing wrong here, but it's a start...

swirt’s picture

swirt
he/him
Primary language English
commented

I added some comments to the MR but they are not showing up here because the MR is not on an issue fork. I'll take a crack at getting the constructor in place.

swirt opened merge request !42

swirt’s picture

swirt
he/him
Primary language English
commented
Status: Active » Needs review

I was wrong, it was not the missing constructor, that is handled correctly by creating the instance. It was that the two static methods did not need to be static. I also cleaned up the perms to handle several perms that allow editing or creating tasks.

jeremylichtman’s picture

jeremylichtman commented

The methods are called statically from burndown.routing.yml. Not sure if that will cause issues.

Otherwise your MR is an improvement on what I threw together.

swirt’s picture

swirt
he/him
Primary language English
commented

Routing is misleading because the class and method looks static with the :: but drupal fully loads the class before calling it. The route works fine being non-static.

jeremylichtman’s picture

jeremylichtman commented
Status: Needs review » Fixed

Released on 1.0.67.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

swirt’s picture

swirt
he/him
Primary language English
commented
Status: Fixed » Closed (fixed)