Problem/Motivation

The "Non-translatable fields can only be changed when updating the original language." validation warning is thrown when content translation is used on a piece of content with a non-translatable Smart Date Reccurring field when the "Hide non translatable fields on translation forms" option is ticked.

It's more obvious when Content Moderation is also enabled for the content since the module enforces the "Hide non translatable fields on translation forms" option.

The validation is triggered by the Drupal core EntityUntranslatableFieldsConstraintValidator:: hasUntranslatableFieldsChanges() call.

And it seems to be because SmartDateWidgetBase attempts to update the default value based on the current RRule whereas EntityUntranslatableFieldsConstraintValidator expects such fields to be untouched since they're hidden from the page.

It's also because the RRule

Steps to reproduce

  • On a site with multiple languages and the #3025039: New non translatable field on translatable content throws error patch applied.
  • Add a non-translatable Smart date range field with a field config similar to the following on a content moderated content type:
    langcode: en
    status: true
    dependencies:
      config:
        - field.storage.node.field_date
        - node.type.event
      module:
        - smart_date
        - smart_date_recur
    third_party_settings:
      smart_date_recur:
        allow_recurring: true
        month_limit: 12
    id: node.event.field_date
    field_name: field_date
    entity_type: node
    bundle: event
    label: Date
    description: ''
    required: false
    translatable: false
    default_value:
      -
        default_duration: 60
        default_duration_increments: "30\r\n60|1 hour\r\n90\r\n120|2 hours\r\ncustom"
        default_date_type: ''
        default_date: ''
    default_value_callback: ''
    settings: {  }
    field_type: smartdate
    
  • Install content translation and enable translation on this content type, but not on the new field date.
  • Ensure "Hide non translatable fields on translation forms" is ticked for the content type before saving.
  • Create an English node content with an all day recurring daily event on 01/08/2023, and ends after 2 times and save as a draft.
  • Create a translation on that content with any arbitrary value e.g. change the title, and also save as a draft
  • Now go back to the original English node, allow it to repeat daily on a specific day, lets say Wednesday and end after 5 times (any day that's different from the first date should suffice), and publish it.
  • Now go back to the translation, and attempt to make a change to the content, e.g. the content title, it should give you the validation error. If no changes are made, then it'll silently ignore it because \Drupal\Core\Entity\ContentEntityBase::hasTranslationChanges() works weirdly and skips the untranslatable smart_date field, but gets triggered by a different field that was also changed

An sqlite database and instructions are available in comment #17

Issue fork smart_date-3194515

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:

Comments

Tichris59 created an issue. See original summary.

tichris59’s picture

Issue summary: View changes
mandclu’s picture

Status: Active » Closed (duplicate)
Issue tags: +ContributionWeekend2021
Related issues: +#3025039: New non translatable field on translatable content throws error

I was able to reproduce the problem, and I agree that it is a serious bug. That said, in my testing the patch at https://www.drupal.org/project/drupal/issues/3025039#comment-13658647 was able to resolve the errors without any changes to Smart Date, so marking this as a duplicate or that issue.

codebymikey’s picture

Issue summary: View changes
Status: Closed (duplicate) » Active

I've unfortunately (depending on the perspective) been able to replicate the issue again, even with the original and latest patches from #3025039: New non translatable field on translatable content throws error.

I've updated the Issue summary accordingly for how to replicate it.

Example code references with similar fixes in place:

I'll upload a patch for what worked for me to the issue for reference purposes tomorrow, but I believe it requires more testing and definitely input from you about how best to go about it (I'd create tests for it, but I'm a bit time-constrained atm)

mandclu’s picture

Based on the entity reference revisions example linked, it seems as though Smart Date needs to create its own FieldItemList implementation, to override the hasAffectingChanges method in particular.

@codebymikey thanks for the additional clarification and i'm very interested to see the patch that worked for you.

mandclu’s picture

Also marking #3269225: Provide a simple method to expand recurring events as related because the changes proposed there might make it easier to implement the necessary hasAffectingChanges (or whatever the fix turns out to be).

codebymikey’s picture

Version: 3.1.x-dev » 3.7.x-dev
StatusFileSize
new6.71 KB

I'm currently on 3.6.1, so I've attached a patch for that here for reference purposes since I don't want to upgrade yet.

But I'll push the equivalent code changes for the 3.7.x-dev branch for us to work from. I don't think it's the cleanest solution, but it works for my use case.

I initially wanted to go the ::hasAffectingChanges() route, since I noticed that we were casting the value, end_value and duration properties to integers when they were actually strings before they were updated, and thought that was the reason the SmartDateFieldItemList::equals() call failed on my first pass.
But on further investigation, I noticed the values were actually different rather that than just being different types, and it was somewhat erroneous since SmartDateFieldItemList::equals() actually does a loose comparison.

I think the Paragraphs approach made the most sense of detecting whether the widget is allowed to automatically make certain changes to the data or not.

codebymikey’s picture

Status: Active » Needs review

Created and updated the issue fork, feedback's more than welcome!

codebymikey’s picture

Version: 3.7.x-dev » 4.0.x-dev
StatusFileSize
new6.74 KB

Updated the patch to support views_bulk_edit integration, there was an issue with the assumption that the form object will always be a \Drupal\Core\Entity\EntityFormInterface which might not always be the case.

mandclu’s picture

Version: 4.0.x-dev » 4.1.x-dev
twcom’s picture

The changes from the most recent patch and merge request aren't working for me. When the EntityUntranslatableFieldsConstraintValidator checks if there are untranslatable changes, it eventually gets down to calling Drupal\Core\Field\FieldItemList::equals to check for changes. At the end of this function, it's returning $value1 == $value2. In my case this evaluates to false (causing the validator to throw an error) because the elements in $value2 have an rrule_index value that $value1 doesn't have.

$value1[0] == [
  duration => "60",
  end_value => 1709226900,
  rrule => "136",
  timezone => "",
  value => 1709223300,
];

$value2[0] == [
  duration => "60",
  end_value => "1709226900",
  rrule => "136",
  rrule_index => "1",
  timezone => "",
  value => "1709223300"
];

$value1 and $value2 both have the same number of elements and, as far as I can tell, the non-rrule_index values are all the same. So it seems like the rrule_index value is causing the check to fail.

I'm on Drupal 10.3.1 and Smart Date 4.1.5 (I rerolled the merge patch locally to get it to apply cleanly, but I didn't change any of the actual code in the MR).

I also tried the patch from #10 and got the same result (had to downgrade my Smart Date to 4.0.0 to get the patch to apply).

Thoughts? Suggestions?

Thanks!

codebymikey’s picture

Attached a static copy of the latest version of the merge request for 4.1.x.

codebymikey’s picture

Relating to #3483466: Recurring date validation targets undefined form elements, ran into a bug with the current implementation where it doesn't properly process the RRules for the recurring dates when they're translated.

Steps to reproduce

1. Create a recurring date with a start and end date of 04/11/2024 01:00 PM - 04/11/2024 02:00 PM.
2. Specify that it repeats weekly on Monday, and has an ends on date of 11/11/2024.
3. Attempt to save the content, it saves just fine.
4. Translate the piece of content, and attempting to save will trigger the warning. This is because the smart date RRule was no longer being processed, but it was still attempting to validate it against the end date of 11/01/2024 12:00 AM which is smaller than the end date instance value of 11/01/2024 02:00 PM.

Proposed resolution

1. We can return early and flag the element as inaccessible with $element['#access'] = FALSE;
2. Load the smart date rules and attempt validations on it even if the field is not actually used (seems like a waste of resources).
3. Update \Drupal\smart_date_recur\Entity\SmartDateRule::validateRecurring() so it does a check if the element is not accessible to the user - if it is not accessible to the user, then it skips any subsequent validations:

if (!Element::isVisibleElement($element)) {
  // Don't attempt to validate recurring if the widget element is not
  // visible on the form - which might be the case for translations.
  // @see \Drupal\content_translation\ContentTranslationHandler::entityFormSharedElements()
  return;
}
mandclu’s picture

Version: 4.1.x-dev » 4.2.x-dev

Updating this to target the 4.2.x branch.

mandclu’s picture

I just tried to reproduce this using the patch posted in #28 of #3025039: New non translatable field on translatable content throws error and an unpatched copy of Smart Date 4.2.x and was unable to reproduce the validation error. Could someone else test this?

chemeon’s picture

Applied !MR 60 as a patch against Smart Date 4.2.0 but it did not resolve my issue - still received a validation error message about untranslatable fields.

Confirming that applying #128 against core 10.3.7 and an unpatched copy of Smart Date 4.2.0 resulted in no validation error on my smart_date field.

chemeon’s picture

Applied !MR 60 as a patch against Smart Date 4.2.0 but it did not resolve my issue - still received a validation error message about untranslatable fields.

Applied #128 against core 10.3.7 and an unpatched copy of Smart Date 4.2.0 and did not receive a validation error.

codebymikey’s picture

Issue summary: View changes
StatusFileSize
new1.39 MB

I've updated the issue summary a bit more to replicate it on a clean install with the #3025039: New non translatable field on translatable content throws error patch.

As well as attached a copy of a 10.3.8 sqlite database to demonstate the behaviour. The relevant settings.php setting is as follows after decompressing it:

$databases['default']['default'] = array (
  'database' => '/var/www/html/web/sites/default/files/.smart_date-3194515.ht_.sqlite',
  'prefix' => '',
  'driver' => 'sqlite',
  'namespace' => 'Drupal\\sqlite\\Driver\\Database\\sqlite',
  'autoload' => 'core/modules/sqlite/src/Driver/Database/sqlite/',
);

And can be viewed in a gitpod instance and triggered by trying to make changes to and save the translation on /ja/node/1/edit.

The patch in #3025039: New non translatable field on translatable content throws error helps but doesn't solve this issue because the "original" field value is still being modified when the RRule is initially read in the translation and causes the \Drupal\Core\Entity\Plugin\Validation\Constraint\EntityUntranslatableFieldsConstraintValidator::hasUntranslatableFieldsChanges() call to return TRUE.