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

Comments

m.stenta created an issue. See original summary.

m.stenta’s picture

m.stenta’s picture

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

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

Version: 2.x-dev » 3.x-dev

m.stenta’s picture

Status: Active » Needs review
m.stenta’s picture

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

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

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

Status: Needs review » Reviewed & tested by the community
m.stenta’s picture

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

Version: 3.x-dev » 4.x-dev
m.stenta’s picture

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

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.