Problem/Motivation

The fact that a log is initially cloned from another log is not preserved in the revision history of the new log. It would be easy to include this, and add useful context to the history of the records.

Proposed resolution

Set the log revision message on newly cloned logs to say "Cloned from log [original-log-id]"

Remaining tasks

Implement.

User interface changes

Revision log of cloned logs will indicate that they were cloned.

API changes

None.

Data model changes

None.

Issue fork log-3296373

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:

  • 3296373-revision-log Comparechanges, plain diff MR !26
    • 2 hidden branches
  • 4.x Comparecompare
  • 3.x Comparecompare
About issue forks

Comments

m.stenta created an issue. See original summary.

m.stenta’s picture

m.stenta
he/him
Primary language English
commented
Related issues: +#3358971: Add link to cloned logs in status message

This is related to but different from: #3358971: Add link to cloned logs in status message

m.stenta’s picture

m.stenta
he/him
Primary language English
commented
Related issues: +#3258268: Add revision message field to actions

Ah I actually forgot we considered this in the past and added a textarea to the confirmation form for an optional revision message in #3258268: Add revision message field to actions. This applied to both the "reschedule" and "clone" actions.

The reasoning was:

When a log is "rescheduled" via the bulk action provided by this module, it would be nice if a revision message could be recorded alongside it. This would provide some visibility into WHY a log is rescheduled, for future reference.

Some thoughts:

  • Providing an optional textarea for the "reschedule" action makes sense, for the reason described above.
  • Providing the same for the "clone" action feels a bit less useful. I think it is helpful to know that a log was cloned from another, but I assume the reason is always pretty simple: "I want a new log like this other one, and this is a shortcut to do that." The cloned log itself is the "why".
  • In practice, we aren't capturing which log was cloned from, and we can't really expect users to do that manually. For one, it's tedious. But more broadly: this is a bulk action, and so it would be impossible to record exactly which logs were the source of the cloned logs (one revision message for many logs).
  • Maybe we should remove the optional revision log message field from the clone log confirmation form?
  • Maybe we should append the optional user-provided revision log message to an explicit default one that we provide? We discussed something like this in #3258268: Add revision message field to actions, but ultimately decided against it.

Proposal:

  • Explicitly set a default revision log message in all actions provided by this module.
  • If the user adds their own message during action confirmation, append that to the end of the default one.
m.stenta’s picture

m.stenta
he/him
Primary language English
commented
Title: Explicitly set revision log on cloned logs » Explicitly set revision log message in bulk actions

Generalizing the title of this issue.

m.stenta’s picture

m.stenta
he/him
Primary language English
commented
Version: 2.x-dev » 3.x-dev

m.stenta opened merge request !26

m.stenta’s picture

m.stenta
he/him
Primary language English
commented
Status: Active » Needs review
m.stenta’s picture

m.stenta
he/him
Primary language English
commented

I don't understand why tests are failing...

    Log Actions (Drupal\Tests\log\Functional\LogActions)
     ✘ Clone single log
       ┐
       ├ A default revision message is set.          
       ├ Failed asserting that two strings are equal.
       ┊ ---·Expected
       ┊ +++·Actual
       ┊ @@ @@
       ┊ -'Cloned·from·qmfv9rt6.'
       ┊ +'Cloned·from·qmfv9rt6.'
       │
       │ /builds/project/log/tests/src/Functional/LogActionsTest.php:59
       ┴
     ✘ Clone multiple logs
       ┐
       ├ The revision message includes a link to the original log and user-provided text.
       ├ Failed asserting that two arrays are equal.                              
       ┊ ---·Expected
       ┊ +++·Actual
       ┊ @@ @@
       ┊  Array (
       ┊ -····0·=>·'Cloned·from·huro6m5g.·Lorem·ipsum.'
       ┊ -····1·=>·'Cloned·from·d619ndfq.·Lorem·ipsum.'
       ┊ -····2·=>·'Cloned·from·jzokzfzl.·Lorem·ipsum.'
       ┊ +····0·=>·'Cloned·from·huro6m5g.·Lorem·ipsum.'
       ┊ +····1·=>·'Cloned·from·d619ndfq.·Lorem·ipsum.'
       ┊ +····2·=>·'Cloned·from·jzokzfzl.·Lorem·ipsum.'
       ┊  )
       │
       │ /builds/project/log/tests/src/Functional/LogActionsTest.php:132
       ┴
m.stenta’s picture

m.stenta
he/him
Primary language English
commented

These tests are passing when I run them locally, so I'm wondering if this is an issue with the Drupal.org Gitlab test runner... 🤔

$ docker compose exec www phpunit /opt/drupal/web/modules/log/tests/
PHPUnit 11.5.55 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.4.18
Configuration: /opt/drupal/phpunit.xml

DDDDDDDDDDDDDDDDDDDDD                                             21 / 21 (100%)

HTML output was generated.
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-387-25979116.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-388-25979116.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-389-25979116.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-390-25979116.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-391-25979116.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-392-25979116.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-393-25979116.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-394-17267934.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-395-17267934.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-396-17267934.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-397-17267934.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-398-17267934.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-399-17267934.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-400-17267934.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-401-45016966.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-402-45016966.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-403-45016966.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-404-45016966.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-405-45016966.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-406-45016966.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-407-45016966.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-408-52379487.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-409-52379487.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-410-52379487.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-411-52379487.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-412-52379487.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-413-52379487.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-414-52379487.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-415-84688786.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-416-84688786.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-417-84688786.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-418-84688786.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-419-84688786.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-420-84688786.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-421-84688786.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-422-84688786.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-423-76197486.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-424-76197486.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-425-76197486.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-426-76197486.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-427-76197486.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-428-76197486.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogActionsTest-429-76197486.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogCRUDTest-48-93887552.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogCRUDTest-49-93887552.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogCRUDTest-50-93887552.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogCRUDTest-51-17791896.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogCRUDTest-52-17791896.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogCRUDTest-53-17791896.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogCRUDTest-54-17791896.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogCRUDTest-55-17791896.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogCRUDTest-56-34360242.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogCRUDTest-57-34360242.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogCRUDTest-58-34360242.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogCRUDTest-59-17770525.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogCRUDTest-60-17770525.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogCRUDTest-61-17770525.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogCRUDTest-62-17770525.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogCRUDTest-63-17770525.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogCRUDTest-64-28607619.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogCRUDTest-65-28607619.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogCRUDTest-66-28607619.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogCRUDTest-67-28607619.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogCRUDTest-68-28607619.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogNamePatternTest-47-31243638.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogNamePatternTest-48-31243638.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogNamePatternTest-49-31243638.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogNamePatternTest-50-31243638.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogNamePatternTest-51-31243638.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogNamePatternTest-52-31243638.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogNamePatternTest-53-51159517.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogNamePatternTest-54-51159517.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogNamePatternTest-55-51159517.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogNamePatternTest-56-51159517.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogNamePatternTest-57-51159517.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogNamePatternTest-58-51159517.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogNamePatternTest-59-57696761.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogNamePatternTest-60-57696761.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogNamePatternTest-61-57696761.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogNamePatternTest-62-57696761.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogNamePatternTest-63-57696761.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogNamePatternTest-64-57696761.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogNamePatternTest-65-57696761.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogNamePatternTest-66-57696761.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogNamePatternTest-67-57696761.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogNamePatternTest-68-57696761.html
http://www/sites/simpletest/browser_output/Drupal_Tests_log_Functional_LogNamePatternTest-69-57696761.html


Time: 09:28.609, Memory: 143.04 MB

OK, but there were issues!
Tests: 21, Assertions: 288, Deprecations: 60, PHPUnit Deprecations: 27.
m.stenta’s picture

m.stenta
he/him
Primary language English
commented

I figured out what was happening. The strings being compared have links in them, which are not represented in the diff I pasted above. I found another diff that shows the full string:

Drupal\Tests\log\Functional\LogActionsTest::testCloneSingleLog
A default revision message is set.
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'Cloned from <a href="/log/1">ybt3fxbg</a>.'
+'Cloned from <a href="/web/log/1">ybt3fxbg</a>.'

/builds/project/log/tests/src/Functional/LogActionsTest.php:59

Pushed a fix that should respect the path prefix being applied in Drupal.org testing environment...

m.stenta’s picture

m.stenta
he/him
Primary language English
commented
Status: Needs review » Reviewed & tested by the community
m.stenta’s picture

m.stenta
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Postponed

I've decided I'm going to wait until #3578407: Start 4.x branch to merge this.

m.stenta’s picture

m.stenta
he/him
Primary language English
commented
Version: 3.x-dev » 4.x-dev
m.stenta’s picture

m.stenta
he/him
Primary language English
commented
Status: Postponed » Reviewed & tested by the community

m.stenta changed the visibility of the branch 3.x to hidden.

m.stenta changed the visibility of the branch 4.x to hidden.

  • m.stenta committed abc187b3 on 4.x 
    Issue #3296373: Explicitly set revision log message in bulk actions
m.stenta’s picture

m.stenta
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

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.

Status: Fixed » Closed (fixed)

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