Contrib.social

Error when trying to revert a group permission revision

Open on Drupal.org →
Created on 3 February 2023, almost 2 years ago
Updated 15 February 2023, almost 2 years ago

Problem/Motivation

When I try to revert to a previous group permission revision, an exception is thrown:

```
TypeError: htmlspecialchars(): Argument #1 ($string) must be of type string, Drupal\Core\Field\FieldItemList given in htmlspecialchars() (line 424 of core/lib/Drupal/Component/Utility/Html.php).

htmlspecialchars(Object, 11, 'UTF-8') (Line: 424)
Drupal\Component\Utility\Html::escape(Object) (Line: 262)
Drupal\Component\Render\FormattableMarkup::placeholderEscape(Object) (Line: 232)
Drupal\Component\Render\FormattableMarkup::placeholderFormat('%log has been reverted to the revision from %revision-date.', Array) (Line: 195)
Drupal\Core\StringTranslation\TranslatableMarkup->render() (Line: 15)
Drupal\Core\StringTranslation\TranslatableMarkup->__toString() (Line: 54)
Drupal\Core\Messenger\Messenger->addMessage(Object, 'status', ) (Line: 73)
Drupal\Core\Messenger\Messenger->addStatus(Object) (Line: 149)
Drupal\group_permissions\Form\GroupPermissionRevisionRevertForm->submitForm(Array, Object)
...
```

This happens on PHP 8.1 and Drupal 9.4.10. The revert apparantly still works. If you go back to the list pf revisions you see a new entry there.

Steps to reproduce

1. View the list of group permission revisions.
2. Click on the "Revert" button next to an older revision.
3. On the next page click on "Revert" again (a message on this page would ne nice btw).

Proposed resolution

In `GroupPermissionRevisionRevertForm::submitForm`, `$this->revision->revision_log_message` is used as if it were a string. It's not, though. Apparently, it's an FieldItemList instance.

The error goes away if `$this->revision->revision_log_message->value` is used instead in line 142 and 148. In line 133 I used `$this->revision->setRevisionLogMessage`, and that seems to work.

🐛 Bug report
Status

Fixed

Version

1.0

Component

Code

Created by

🇩🇪Germany ammaletu Bonn, Germany

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

Comments & Activities

  • Issue created by @ammaletu
  • Comment almost 2 years ago →
    🇩🇪Germany ammaletu Bonn, Germany
  • Assigned to lobsterr
  • Comment almost 2 years ago →
    🇧🇪Belgium lobsterr
  • @lobsterr opened merge request.
  • Status changed to Needs review almost 2 years ago
  • Comment almost 2 years ago →
    🇧🇪Belgium lobsterr

    @Ammaletu - It was fixed. I also improved a messages and reverted log message. Please check it and I will tag a new version

  • Status changed to Needs work almost 2 years ago
  • Comment almost 2 years ago →
    🇩🇪Germany ammaletu Bonn, Germany

    I tested your changes. Reverting the permission revision works now. There is something strange about the message which is set, though. I got `Copy of the revision Test 1 from 46.`. The last part is supposed to be the date, but only shows the minute of the original revision which was reverted (saved at 18:46). I tried with different revisions and always got the minute from the original timestamp.

  • Comment almost 2 years ago →
    🇧🇪Belgium lobsterr

    I have done some improvements, but I can't reproduce your issues. In the clean Drupal installation I get such message:

    my revision (Sat, 02/11/2023 - 01:36) has been reverted to the revision from new revision (Sat, 02/11/2023 - 01:35).

    I am not using specific date format here, it comes from the system configuration:
    $this->dateFormatter->format($current_revision->getRevisionCreationTime())

    I assume probably for your Drupal installation you have date format, which displays only time.

    Could you please check it ?

  • Comment almost 2 years ago →
    🇩🇪Germany ammaletu Bonn, Germany

    I checked this again and debugged it a bit. The date formatter correctly outputs `Di., 14.02.2023 - 18:49` for the current timestamp. When I use this as input for the t function's placeholder `:date`, only the 49 is in the output. Everything works as expected when using `@date` as placeholder.

    The documentation for the t function placeholders list the different approaches:
    https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Component%21Rend...

    It states for : placeholders: "Return value is escaped with \Drupal\Component\Utility\Html::escape() and filtered for dangerous protocols using UrlHelper::stripDangerousProtocols()." The latter function interpretes everything before the minute as a protocol, and since it is unknown it is stripped out. I think in this case using @ for the placeholders is the better approach.

  • Comment almost 2 years ago →
    System Message
    • LOBsTerr → committed 32b36aee on 1.0.x 
      Issue #3338926 by LOBsTerr: Error when trying to revert a group...
  • Status changed to Fixed almost 2 years ago
  • Comment almost 2 years ago →
    🇧🇪Belgium lobsterr

    Nice catch. I strange that I wasn't able to reproduce it. Anyway, I want to thank you for your contribution and your time.

  • Comment almost 2 years ago →
    🇩🇪Germany ammaletu Bonn, Germany

    No problem. The company I work at is using Drupal for our product, so giving back to the community a bit feels good. Thanks for creating the new release!

  • Comment over 1 year ago →
    System Message

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

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024