Logic in code doesn't follow the logic in settings.

Created on 23 July 2020, over 4 years ago
Updated 31 May 2024, 7 months ago

Hi there,

when I was setting up the module I figured out, that some settings did not take effect ,although there was no reason for that.

Steps to reproduce

Sub-issue 1

  • Go to module settings at /admin/config/content/content-notify
  • Set up the first tab "Notify user of old content" and nothing else.
  • Email recipient will never get an email, because there is an extra condition in the NotifyExtendForm.php at line 170 if (!empty($entity->unpublish_on->value)).

This condition is not necessary, as this feature should only check if a content is old, it has nothing to do with scheduling.

Sub-issue 2

In our use case, we are using the workflow module, but don't want use the workflow settings in Content notification. So the Criteria variable (set on line 559) is still FALSE at line 592. Then the next code hunk "syncing the unpublished time" is run because of it. And at the end of this hunk is a return so nothing more will get ever evaluated.
I don't think the return should be there. Next parts are independent and should run as well.
Maybe my fix is not the correct one, but there is something in the logic which seems not exactly right.

  • Go to module settings at /admin/config/content/content-notify
  • Set up the first tab "Notify user of old content" and nothing else.
  • Email recipient will never get an email, because there is a return in content_notify.module at line 608.

Patch for both sub-issues will follow.

πŸ› Bug report
Status

RTBC

Version

2.0

Component

Code

Created by

πŸ‡¨πŸ‡ΏCzech Republic Petr Illek

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

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡¬πŸ‡§United Kingdom andy_w

    Currently this module works fine if workflows module is not used, or if the content notify date is set manually in the field.

    However the default date is ignored currently due to the erroneous return (as highlighted above) if the workflows module is enabled but transitions are not used as part of the module configuration.

    This patch does not go back and rectify old data, if you had been using this module with existing data this patch will not fix existing data, but will allow the default to be populated on new and updated content going forward.

  • Status changed to Needs review 27 days ago
  • πŸ‡ΊπŸ‡ΈUnited States jrb Raleigh-Durham Area, NC, USA

    We definitely do not want to just remove that return from content_notify_node_presave() because the code below that section should only run if the node is published. I think the problem is just that the logic in the conditionals that set $criteria is wrong and inconsistent in how it behaves with moderated and unmoderated content.

    As noted, if the Workflows module is enabled, it only looks that the moderation state, regardless of whether that content type has moderation enabled (or if that's what is wanted). The current logic also only works if there was a previous status AND that status changed. So, if moderation is used and content is directly published or currently-published content is edited and kept published, it will be ignored. This is different from the logic used when Workflows is not enabled. In that case, it simply looks to see if the content is published.

    I've attached a patch that makes the logic consistent, regardless of whether the Workflows module is enabled or not. With this, it checks:

    1. Is "Use transition criteria" checked?
    2. Is the Workflows module is enabled?
    3. Does the content type of the current node actually use moderation?

    - If all of those are true, it then checks to see if the current moderation state matches the "To state" setting.
    - If any of those are false (e.g. if you don't want to use the transition criteria or moderation isn't used for this content type), then it does not look at moderation state and uses the "Published" status instead.

    The patch also updates the description of the "Use transition criteria" setting so it's clear how it works (per above).:

    If not checked, the criteria is just that the node being saved is "Published". If checked, the criteria is that the node being saved is in the state specified below.
    Note: Content types that do not use a moderation workflow will ignore this setting and use the "Published" status only.

Production build 0.71.5 2024