Problem/Motivation

content_translation.admin.inc just holds a bunch of helpers for a form alter.
Let's move the form alter to it's own class and move the helpers

Steps to reproduce

Open the file

Proposed resolution

Move the form alter to it's own class and move helpers.
Create second form alter class to reduce the number of injected dependencies.
Deprecate inc file
Create service for content_translation_field_sync_widget since it is reused and deprecate.
Move deprecated function to module file.
Remove loadfile calls
Add DI
Use string translation trait
Move preprocess helper inline -> Add DI here in a follow up

Remaining tasks

Review with Zebra:
Using Zebra you can easily see the changes:
git diff 11.x --color-moved=dimmed_zebra --color-moved-ws=ignore-all-space

User interface changes

N/A

Introduced terminology

N/A

API changes

New FieldSyncWidget service.

Data model changes

N/A

Release notes snippet

N/A

Issue fork drupal-3548571

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:

About issue forks

Comments

nicxvan created an issue. See original summary.

nicxvan opened merge request !13311

nicxvan’s picture

nicxvan commented

Let's see what tests and stan say, I think I need to update some coverage attributes.

I deprecated the file and the one function that needs it.

nicxvan’s picture

nicxvan commented

Mistakenly removed string translation trait so I added it back and regenerated baseline since this is already large so functional changes are out of scope.

A zebra scan should make this review far easier too:

git diff 11.x --color-moved=dimmed_zebra --color-moved-ws=ignore-all-space

nicxvan’s picture

nicxvan commented

Not sure why the local generation failed, using the one in the job.

nicxvan’s picture

nicxvan commented
Issue summary: View changes
Status: Active » Needs review

This is ready for review!

While this might be a bit bigger than expected it's just moving the functions while adding DI $this->t and one new service to prevent duplication and allow deprecation.

Using Zebra you can easily see the changes:
git diff 11.x --color-moved=dimmed_zebra --color-moved-ws=ignore-all-space

nicxvan’s picture

nicxvan commented
Issue summary: View changes
nicxvan’s picture

nicxvan commented
Issue summary: View changes
berdir’s picture

berdir
Primary language German
Location Switzerland
commented

reviewed a bit. since this also does non form alter stuff, maybe retitle to remove and deprecate functions from content_translation.admin.inc or so?

nicxvan’s picture

nicxvan commented
Title: Clean up content translation form alters » Remove and deprecate functions from content_translation.admin.inc

Good suggestion

nicxvan’s picture

nicxvan commented
Issue summary: View changes

I think I've addressed all of your feedback except one comment I think should be in a follow up.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Needs work

Commented.

deepakkm made their first commit to this issue’s fork.

deepakkm’s picture

deepakkm commented
Status: Needs work » Needs review
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Reviewed & tested by the community

Thanks, I think this is ready then. fairly straightforward and another .inc file dealt with.

nicxvan’s picture

nicxvan commented

Fair enough!

For some reason I thought there were more services.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

I merged the suggested changes for the 11.3/4 that @catch suggested, including an additional one for the .inc file.

nicxvan’s picture

nicxvan commented

Changes look good, I searched the MR and see no other 11.3 deprecations.

quietone’s picture

quietone commented
Title: Remove and deprecate functions from content_translation.admin.inc » Deprecate functions from content_translation.admin.inc
Status: Reviewed & tested by the community » Needs work

I left 2 comment that need attention. I did not review the changes, I just looked at the deprecation messages. So, simple change.

Changing title because deprecation is the first step. I removed the 'remove' because removing the entire file is the next step. I thought this would be part of the meta about removing include files but when I finally found that one it is about core/includes only.

Edit: I read that issue incorrectly.

nicxvan’s picture

nicxvan commented
Status: Needs work » Needs review

I updated both of those things! I'll keep an eye on tests.

quietone’s picture

quietone commented
Status: Needs review » Reviewed & tested by the community

Thanks, back to RTBC

  • catch committed 709a9bbd on 11.x 
    task: #3548571 Deprecate functions from content_translation.admin.inc...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

After making those suggestions I went back and forth on whether this should just go into the 11.3 during beta, but I think this is fine to land only in 11.4 with the deprecations for removal in 12.x - we don't expect anyone to be calling any procedural functions from content_translation.admin.inc.

Committed/pushed to 11.x, thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

catch closed merge request !13311

Status: Fixed » Closed (fixed)

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

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented

This change deleted function content_translation_form_language_content_settings_submit(), which is used by Webform testing. Since this function name does not start with an underscore, I thought it wouldn't be removed until a major version update. This change has lead to #3568000: Replace use of content_translation_form_language_content_settings_submit().