Hi,

It was discussed on the documentation list that this would be a useful feature for both documentation and user sites. This patch adds a new option to the workflow form called "Require log messages". If this is enabled, the textarea is set to required, and the fieldset is expanded. Otherwise, nothing else is changed.

Enjoy!
--Andrew

CommentFileSizeAuthor
#2 308352_allow_required_log_messages_2.patch2.1 KBdeviantintegral
allow_required_log_messages.patch2.02 KBdeviantintegral

Issue fork drupal-308352

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

Status: Needs review » Needs work

Patch failed to apply. More information can be found at http://testing.drupal.org/node/14354. If you need help with creating patches please look at http://drupal.org/patch/create

deviantintegral’s picture

deviantintegral commented
Status: Needs work » Needs review
StatusFileSize
new 308352_allow_required_log_messages_2.patch2.1 KB

Bah, made the patch against the node folder instead of the drupal root.

add1sun’s picture

add1sun commented

Looks good to me. I haven't had a chance to test it yet. My only nit-picky thing (since this is core) is that comments should end in a period.

oadaeh’s picture

oadaeh commented
Status: Needs review » Needs work

Your second patch adds a line with nothing more than two spaces.

I'm also attempting to review it more in depth, but I'm currently trying to get past http://drupal.org/node/299661.

Everything looks fine, but I want to verify one validity check.

oadaeh’s picture

oadaeh commented

Okay, as I suspected, there are not enough validation checks being done. Here are the things I found wrong with the proposed patch:

  • If 'Require log messages' is enabled for a content type, a log message is required for both the creation of a content as well as its deletion, which should not be.
  • If 'Require log messages' is enabled for a content type but 'Create new revision' is not enabled (for either the content type or the content being edited), the log messsage is saved, but one can never view it, because only the current revision is saved in the database.
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

Also, not to sound like a broken record or anything, but we'll need tests for this. There's a lot of tweaky logic, as evidenced by oadaeh's #5. In fact, this might be a great case where "test-driven development" (writing tests that document the logic first, then filling in code so they pass) could shine.

jbrauer’s picture

jbrauer commented

http://drupal.org/node/120207 marked as a duplicate... though it was the predecessor since the patches/work is here keeping this one.

aren cambre’s picture

aren cambre commented

subscribing as author of predecessor request

deviantintegral’s picture

deviantintegral commented

If 'Require log messages' is enabled for a content type, a log message is required for both the creation of a content as well as its deletion, which should not be.

So I traced this down. The problem is that node_validate is being called by the edit form when you hit the delete button, validating all of the filled in form elements. Since any changes will be lost when you click the delete button, can't validation just be removed for that case? Or am I missing something more complex?

If 'Require log messages' is enabled for a content type but 'Create new revision' is not enabled (for either the content type or the content being edited), the log messsage is saved, but one can never view it, because only the current revision is saved in the database.

This brings up a more interesting case. I suppose it's possible to want to force log messages without revisions. If that's a valid case, then I guess the 'Revisions' tab could be changed to a 'Log messages' tab when revisions are disabled. Otherwise, the workflow form can be changed so that the option is only available if revisions are enabled.

As for doing tests, I totally agree, but unfortunately I'm rather new at testing in general. I'll take this as an opportunity to learn and see what I can come up with :).

--Andrew

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

@deviantintegral: Awesome! Don't hesitate to stop by #drupal and ask testing questions if you get stuck. There are lots and lots of people who would love to help you out. :)

Susurrus’s picture

Susurrus commented

#265259: Node form is validated before deletion should probably be resolved before this then.

oadaeh’s picture

oadaeh commented

This brings up a more interesting case. I suppose it's possible to want to force log messages without revisions. If that's a valid case, then I guess the 'Revisions' tab could be changed to a 'Log messages' tab when revisions are disabled. Otherwise, the workflow form can be changed so that the option is only available if revisions are enabled.

AFAIK, there is no way to view the saved log message if revisions are disabled. Saving them is fine, but kind of pointless w/o a UI.

aren cambre’s picture

aren cambre commented
Status: Needs work » Active

Patch is so old at this point that work here certainly needs a reset.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Version: 7.x-dev » 8.x-dev

New features go into 8.x.

colan’s picture

colan
he/him
Primary language English
Location Toronto 🇨🇦
commented
Issue summary: View changes

Until somebody adds this to core, there's Enforce revision log message in contrib.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

matsbla’s picture

matsbla commented

I created this issue #2978701: Replace the revision log message with a comment field. I'm not sure if this is a solution for this issue, but if revision log messages were a comment field we could at least decide which comment fields are mandatory (and by that if a message is requiered or not).

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

dpi’s picture

dpi
Location Perth, Australia
commented
Component: node system » entity system

Revisions no longer bound to node

brayfe’s picture

brayfe commented

Thanks for the lead in #15. Was hoping I could do this with a core feature, without writing custom code, but I'll take a contrib module. Hope to see this in core in the future!

heddn’s picture

heddn
Primary language English
Location Nicaragua
commented

The extent of a "custom module" that is needed is:

function my_module_entity_base_field_info_alter(&$fields, EntityTypeInterface $entity_type) {
  if ((isset($fields['revision_log']) && $revision_log = 'revision_log') || (isset($fields['revision_log_message']) && $revision_log = 'revision_log_message')) {
    $fields[$revision_log]->setRequired(TRUE);
  }
}
hchonov’s picture

hchonov
Primary language Bulgarian
Location Burgas
commented

You don't need to hard code the name of the field as of https://www.drupal.org/node/2831499 .

function my_module_entity_base_field_info_alter(&$fields, EntityTypeInterface $entity_type) {
  $revision_log_field = $entity_type->getRevisionMetadataKey('revision_log_message');
  if ($revision_log_field && isset($fields[$revision_log_field])) {
    $fields[$revision_log_field]->setRequired(TRUE);
  }
}

Providing a configurable core feature for that however would require a more general solution. We would need to provide configurable options for all base fields of all entity types.

I just found this module - https://www.drupal.org/project/base_field_override_ui

I don't know if it exposes the revision log field, but as this allows overrides on bundle level I think this is the way to go. If this should be a core feature I would consider this module for inclusion in core or implementing something similar.

heddn’s picture

heddn
Primary language English
Location Nicaragua
commented

Actually in my testing, I had to hard code because calling getRevisionMetadataKey results in a circular endless loop. Which makes sense. But using those names is also how I stumbled on how core does it too in at least one place. So it seems that, at least for core provided revision log fields, we have all our base (fields) covered. Pun intended.

Good find in https://www.drupal.org/project/base_field_override_ui. If I didn't have a bunch of places to change this, that might make more sense.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev
crutch’s picture

crutch commented

Ultimately we would like to have this. example

crutch’s picture

crutch commented
Version: 9.3.x-dev » 9.4.x-dev

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Bhanu951 made their first commit to this issue’s fork.

bhanu951’s picture

bhanu951 commented
Related issues: +#3153559: Switch Node revision UI to generic UI, +#2723579: NodeRouteProvider should extend DefaultHtmlRouteProvider
bhanu951’s picture

bhanu951 commented
Related issues: +#2350939: Implement a generic revision UI

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.