Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EWPP-2646: Fix publication thumbnail translation render. #1174

Merged
merged 1 commit into from
Nov 3, 2022
Merged

Conversation

nagyad
Copy link
Member

@nagyad nagyad commented Oct 4, 2022

No description provided.

@nagyad nagyad force-pushed the EWPP-2646 branch 2 times, most recently from 0606baf to 256a679 Compare October 7, 2022 15:31
sergepavle
sergepavle previously approved these changes Oct 7, 2022
*/
public function testPublicationThumbnailMediaTranslation(): void {
// Make publication node and image media translatable.
\Drupal::service('content_translation.manager')->setEnabled('node', 'oe_publication', TRUE);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bundle is already translatable because this is done in the setUp() method.

/**
* Tests that the Publication thumbnail media renders the translated media.
*/
public function testPublicationThumbnailMediaTranslation(): void {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't we test this in the previous test method? The title field is set to be translatable in the setUp() and then the translation of the node is being tested. We can just do all the configuration needed in the setUp() and then provide a Thumbnail for the publication node so we can test this scenario too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because that method is over 400 lines. I don't like shovelling everything under one test case. Here we test a specific scenario and this scenario is separate for event as well.

22Alexandra
22Alexandra previously approved these changes Oct 13, 2022
sergepavle
sergepavle previously approved these changes Oct 13, 2022
@nagyad nagyad merged commit 4dd21e4 into 3.x Nov 3, 2022
@nagyad nagyad deleted the EWPP-2646 branch November 3, 2022 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants