Problem/Motivation
We should have the ability to choose the status of new translations when they are downloaded into Drupal 8. The status should be Same as source by default (that's what we do now), but we should be able to set up Published or Unpublished for now, and in 8.3.x we may need to expand that if content moderation goes into core.
Proposed resolution
Add a setting on the LingotekSettingsTabPreferencesForm that allows the ability to choose what status translations will be downloaded in.
The logic for doing this is added into the saveTargetData() in the LingotekContentTranslationService. This can only occur if the Publishing Status is a translatable field and so that is checked first and if it is then it continues and checks the new setting. If it is set to "1"=>"Published" then it will download the new translation and set the status to published. If set to "0"=>"Unpublished" then the status will be set to unpublished and if set to "2"=>"Same as source" then it will check the status of the source and set it to that.
Remaining tasks
None
User interface changes
Added a preference on the LingotekSettingsTabPreferencesForm that has a dropdown for the options and a description to accompany the dropdown.
API changes
Added:
LingotekConfigurationServiceInterface::getPreference($preference_id)
LingotekConfigurationServiceInterface::setPreference($preference_id, $value);
Data model changes
None
Comment | File | Size | Author |
---|---|---|---|
#61 | 2832516-published-setting-61.patch | 14.21 KB | penyaskito |
| |||
#61 | 2832516-published-setting-59-61.interdiff.txt | 2.1 KB | penyaskito |
#59 | 2832516-published-setting-59.patch | 14.35 KB | penyaskito |
| |||
#59 | interdiff.txt | 13 KB | penyaskito |
#57 | 2832516-published-setting-57.patch | 11.36 KB | penyaskito |
Comments
Comment #2
t.murphy CreditAttribution: t.murphy at Lingotek commentedHere is the patch that contains the suggested changes.
Comment #3
t.murphy CreditAttribution: t.murphy at Lingotek commentedComment #5
t.murphy CreditAttribution: t.murphy at Lingotek commentedFixed the two errors that occurred during the test of the last patch. The preference needed to be added to the lingotek.settings and it appeared that there was a problem with the description for the new preference.
Comment #6
t.murphy CreditAttribution: t.murphy at Lingotek commentedComment #8
t.murphy CreditAttribution: t.murphy at Lingotek commentedThe new preference did not have a default value and so it was causing errors in the test. I added a default value.
Comment #9
t.murphy CreditAttribution: t.murphy at Lingotek commentedComment #11
t.murphy CreditAttribution: t.murphy at Lingotek commentedComment #12
t.murphy CreditAttribution: t.murphy at Lingotek commentedComment #14
t.murphy CreditAttribution: t.murphy at Lingotek commentedA new patch to attempt the fix for the Undefined Index.
Comment #15
t.murphy CreditAttribution: t.murphy at Lingotek commentedComment #17
penyaskitoRe-testing, one of the failures is related to #2840990: Fix LingotekDashboardTest::testTranslationsAvailable() for compatibility with 8.3.x. The other one maybe is gone?
Comment #18
penyaskitoThis is the same failures as #2840990: Fix LingotekDashboardTest::testTranslationsAvailable() for compatibility with 8.3.x, so we are good.
However, we should add new tests for the new behaviours. Also, some improvements we should probably make:
This should be a setting available for any content that have a status, not only nodes. Could we use a different name that doesn't tie us to nodes? Maybe have a settings per entity type (even bundle?)?
Let's use moderation states if content_moderation is enabled. For that, we should be using strings and get the available states from the content_moderation module data.
These changes are unneeded?
Comment #19
t.murphy CreditAttribution: t.murphy at Lingotek commentedComment #20
t.murphy CreditAttribution: t.murphy at Lingotek commentedComment #22
t.murphy CreditAttribution: t.murphy at Lingotek commentedComment #23
t.murphy CreditAttribution: t.murphy at Lingotek commentedComment #25
t.murphy CreditAttribution: t.murphy at Lingotek commentedComment #26
t.murphy CreditAttribution: t.murphy at Lingotek commentedComment #28
t.murphy CreditAttribution: t.murphy at Lingotek commentedComment #29
t.murphy CreditAttribution: t.murphy at Lingotek commentedComment #31
t.murphy CreditAttribution: t.murphy at Lingotek commentedComment #33
t.murphy CreditAttribution: t.murphy at Lingotek commentedComment #35
t.murphy CreditAttribution: t.murphy at Lingotek commentedComment #37
t.murphy CreditAttribution: t.murphy at Lingotek commentedComment #39
t.murphy CreditAttribution: t.murphy at Lingotek commentedComment #41
t.murphy CreditAttribution: t.murphy at Lingotek commentedComment #43
t.murphy CreditAttribution: t.murphy at Lingotek commentedComment #45
t.murphy CreditAttribution: t.murphy at Lingotek commentedComment #47
t.murphy CreditAttribution: t.murphy at Lingotek commentedComment #49
t.murphy CreditAttribution: t.murphy at Lingotek commentedComment #50
t.murphy CreditAttribution: t.murphy at Lingotek commentedComment #53
t.murphy CreditAttribution: t.murphy at Lingotek commentedComment #55
t.murphy CreditAttribution: t.murphy at Lingotek commentedComment #57
penyaskitoFixed syntax error.
Comment #59
penyaskitoThe problem is that we are using the lingotek service for accessing the configuration, but we have that mocked in the tests. So the feature was working, the test wasn't.
I'm changing that getting rid of the $this->lingotek->get call, which we should stop using unless actually calling the service, but that it's a cleanup that should happen in other issue.
This patch is green and has more scenarios coverage, but I want to still clean it a bit.
Comment #60
penyaskitoLet's remove this commented piece of code.
Oops, I forgot this while debugging. Let's remove it.
We need a #title for this, looks ugly.
Comment #61
penyaskitoAttended feedback at #60. Also added a default value for the new setting in our config for new installations.
Comment #62
penyaskitoUpdated issue summary with API addition.
Comment #63
penyaskitoDiscussed with @t.murphy and this is RTBC. Tests passed as expected now :-)
Comment #65
penyaskitoCommitted 639868d and pushed to 8.x-1.x. Thanks!