In this change record, the example with allowedIf() is actually wrong, I think. Since the AccessResultAllowed class doesn't have a setReason() method, the example code would lead to a fatal error whenever access is allowed.
Instead maybe something like this:

$access = AccessResult::allowedIf($account->hasPermission('access comments') && $entity->isPublished())
      ->cachePerPermissions()
      ->addCacheableDependency($entity);
    if ($access instanceof AccessResultReasonInterface) {
      $access->setReason("The 'access comments' permission is required and the comment must be published.");
    }
    return $access;

if this is changed, it would actually also be good, I think, if the change record could mention that (and why) the reason shouldn't be translated. That's a bit confusing to me.

Finally: Is this the right way to report mistakes in (or suggestions for) change records? Or are comments monitored usually?

Comments

drunken monkey created an issue. See original summary.

cilefen’s picture

I suggest, in order of preference:

  1. Edit the change record.
  2. Discuss the problem on the issue related to the change record.
  3. Comment on the change record.
cilefen’s picture

Status: Closed (works as designed) » Active

I did not really mean to close this, but we probably should.

Wim Leers’s picture

#2++

Also, yes, your suggested example is correct, the example in the CR is indeed wrong for the reasons you cite. Do you want to fix it, or shall I?

Thanks for reporting this!

drunken monkey’s picture

Category: Bug report » Support request
Status: Active » Fixed

Sorry, I didn't realize I could edit this myself! I naturally looked (since you can edit most nodes on d.o), but apparently I was logged out at the time and didn't realize it. Sorry for the unnecessary issue, and thanks for answering my question (and agreeing with my complaint)!

Wim Leers’s picture

Haha, no problem :)

Thanks again for taking the time to rectify this! Much appreciated :)

P.S.: looking forward to your feedback on using D8's REST module. Feel free to ping me on IRC to share your experience, publicly or in PM!

Wim Leers’s picture

Category: Support request » Task

Status: Fixed » Closed (fixed)

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