Problem/Motivation

As pointed out by Upchuk in comment https://www.drupal.org/project/commerce_shipping/issues/3248745#comment-...
the system reports Mismatched entity and/or field definitions after https://www.drupal.org/node/3248745 has been committed.
(I cannot re-open #3248745: Add "created" and "changed" timestamps to shipping method entities hence this new issue.)

Steps to reproduce

  • Have a system on 8.x-2.2 with commerce_shipping_method entities
  • Have content translation enabled on shipping methods
  • Upgrade to commerce_shipping 8.x-2.3
  • Check the status page at admin/reports/status

Proposed resolution

A simple way to fix this is removal of the now obsolete field storage definitions.

NB: There is no data migration to the new fields. Since 8.x-2.3 has already been released the data in the old fields are no longer maintained anyway though.

Data model changes

Removal of "content_translation_created" and "content_translation_changed" field storage definitions. The field definitions are already gone due to the core not adding them if created and changed are present.

CommentFileSizeAuthor
#12 interdiff_11-12.txt289 bytesnarendra.rajwar27
#12 3263586-12.patch891 bytesnarendra.rajwar27
#11 3263586-11.patch919 bytesalexdoma
#10 3263586-10.patch1.79 KBalexdoma
#8 3263586-8.patch1.79 KBalexdoma
#2 3263586-remove-stale-field-storage-definitions.patch1.46 KBcspitzlay

Comments

cspitzlay created an issue. See original summary.

cspitzlay’s picture

cspitzlay
Primary language German
Location 🇩🇪🇪🇺
commented
Status: Active » Needs review
StatusFileSize
new 3263586-remove-stale-field-storage-definitions.patch1.46 KB
upchuk’s picture

upchuk commented

Hey there, i had already created this followup https://www.drupal.org/project/commerce_shipping/issues/3262879 but forgot to mention in that other issue. I will close mine.

upchuk’s picture

upchuk commented
Status: Needs review » Needs work

I'm not sure this is a correct approach because it will remove data accumulated from before adding the created/changed fields. It should be copied over at least IMO because this will leave it will empty created/changed values where before we used to have translation created/changed values.

mkalkbrenner’s picture

mkalkbrenner
Primary language German
Location 🇩🇪
commented
Issue summary: View changes

In my opinion this is a bug in core. If content_translation adds fields it it also responsible for removing the fields if the condition for their existence no longer exists.

Anyway, copying the old values is dangerous. You should not do it in an update hook, but in a post update. But what happens if you save the shipping method (and all its translations.) Won't that modify the changed date immediately?
Does it trigger any APIs and potentially custom code you don't wont to have executed?

And content translation created doesn't exactly mean the same as entity created if content translation has been installed later.

What about revisions?

I think that simply deleting the fields is the only reliable patch to not open more bugs and to avoid unexpected side effects.
Having "wrong" dates after the deletion might be the least serious issue. Especially if you consider that this has never been a feature of this module itself.

calbasi’s picture

calbasi
Primary language Catalan
Location Catalonia
commented

Your patch #2 not running in my side:

vendor/bin/drush updb
PHP Fatal error: Cannot redeclare commerce_shipping_update_8206() (previously declared in .../web/modules/contrib/commerce_shipping/commerce_shipping.install:130) in .../web/modules/contrib/commerce_shipping/commerce_shipping.install on line 137
[warning] Drush command terminated abnormally.

c_archer’s picture

c_archer commented
Related issues: +#3308423: The Translation created time field needs to be uninstalled.

I've seen the same issue, I had raised a new ticket (https://www.drupal.org/project/commerce_shipping/issues/3308423) but closed as it appears to be similar if not the same as this one.

alexdoma’s picture

alexdoma commented
StatusFileSize
new 3263586-8.patch1.79 KB

update a patch

alexdoma’s picture

alexdoma commented
alexdoma’s picture

alexdoma commented
StatusFileSize
new 3263586-10.patch1.79 KB

add correct patch

alexdoma’s picture

alexdoma commented
StatusFileSize
new 3263586-11.patch919 bytes

This patch generated by linux. Previous patch generated by Windows(powershell) not applied. reason - encoding

narendra.rajwar27’s picture

narendra.rajwar27
he/him
Primary language Hindi
Location India
commented
Status: Needs work » Needs review
StatusFileSize
new 3263586-12.patch891 bytes
new interdiff_11-12.txt289 bytes

Adding patch. by fixing CS issue.

c_archer’s picture

c_archer commented

The patch in #11 resolved the issue for me.

  • jsacksick committed 398bef8 on 8.x-2.x 
    Issue #3263586 by alexdoma, narendra.rajwar27, cspitzlay: Mismatched...
jsacksick’s picture

jsacksick commented
Status: Needs review » Fixed

Committed, thanks everyone!

Status: Fixed » Closed (fixed)

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