Problem/Motivation

In #2679008: Module weight is not taken into account after module installation weight of the content_translation module changed in some scenarios. We need to ensure our form alter for language content settings runs after content translation module acted on that one.

Proposed resolution

Increase weight of lingotek module.

Remaining tasks

None

User interface changes

None

API changes

None.

Data model changes

None

CommentFileSizeAuthor
#15 2892220-module-weight-15.patch231.62 KBpenyaskito
#15 2892220-module-weight-15.only-tests.patch230.38 KBpenyaskito
#8 2892220-module-weight-8.no-binary.txt3.67 KBpenyaskito
#8 2892220-module-weight-8.patch233.36 KBpenyaskito
#8 2892220-module-weight-8.only-tests.patch232.13 KBpenyaskito
#5 2892220-module-weight-2.patch791 bytespenyaskito
#2 2892220-module-weight-2.patch791 bytespenyaskito

Comments

penyaskito created an issue. See original summary.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
StatusFileSize
new 2892220-module-weight-2.patch791 bytes

We will need a hook_update_N for that.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
Status: Active » Needs review
penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
Version: 8.x-1.x-dev » 8.x-2.x-dev
penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
StatusFileSize
new 2892220-module-weight-2.patch791 bytes

re-uploading

The last submitted patch, 2: 2892220-module-weight-2.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 5: 2892220-module-weight-2.patch, failed testing. View results

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
Status: Needs work » Needs review
StatusFileSize
new 2892220-module-weight-8.only-tests.patch232.13 KB
new 2892220-module-weight-8.patch233.36 KB
new 2892220-module-weight-8.no-binary.txt3.67 KB

Upgrade path and upgrade path tests.

The last submitted patch, 8: 2892220-module-weight-8.only-tests.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 8: 2892220-module-weight-8.patch, failed testing. View results

clemens.tolboom’s picture

clemens.tolboom
Primary language Dutch
Location Groningen, 🇳🇱/🇪🇺
commented 
+++ b/lingotek.install
@@ -10,9 +10,18 @@ use Drupal\Core\Field\FieldStorageDefinitionInterface;
+  module_set_weight('lingotek', 15);

// $weight = module_get_weight('content_translation');

$weight = 15;

module_set_weight('lingotek', $weight);

15 is an arbritrary number. I would anticipate on module_get_weight('content_translation'); which is (or was?) not implemented. Then write a test for when core fixes this module_set_weight.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented

module_get_weight doesn't exist, and content_translation already has an arbitrary weight (10).
I'm including tests covering the upgrade + a new install. There may be cases where content_translation weight will be 0, as there was no upgrade path on that issue. I'm commenting about that there with further analysis.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented

Dump needs to be done with lower supported version (8.3.x) or they may be schema errors in the tests as its the case.

clemens.tolboom’s picture

clemens.tolboom
Primary language Dutch
Location Groningen, 🇳🇱/🇪🇺
commented

I should have added #1808132: Move module_set_weight() into ModuleHandler::setWeight(), add ModuleHandler::getWeight() to replace missing functionality

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
Status: Needs work » Needs review
StatusFileSize
new 2892220-module-weight-15.only-tests.patch230.38 KB
new 2892220-module-weight-15.patch231.62 KB

Thanks, @clemens.tolboom.

Attaching new patch with new dump. No interdiff as it's a binary change.

The last submitted patch, 15: 2892220-module-weight-15.only-tests.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 15: 2892220-module-weight-15.patch, failed testing. View results

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
Status: Needs work » Reviewed & tested by the community

Tests passed and failed as expected (well, aside of the testbot issue writing to disk in random occasions)

  • penyaskito committed ad06061 on 8.x-2.x 
    Issue #2892220 by penyaskito: Change weight of the lingotek module after...
penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
Status: Reviewed & tested by the community » Fixed

Committed ad06061 and pushed to 8.x-2.x. Thanks!

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented

Status: Fixed » Closed (fixed)

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