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

EZEE-2661: Allowed translation filtering when publishing version #2615

Merged
merged 6 commits into from
May 30, 2019

Conversation

ViniTou
Copy link
Contributor

@ViniTou ViniTou commented Apr 23, 2019

Question Answer
JIRA issue EZEE-2661
Bug/Improvement yes
New feature yes
Target version 7.5 (2.5.1)
BC breaks no
Tests pass yes
Doc needed yes

As a part of fix, we decided to add possibility to publish specific translations only.
As it seems more clearer now, I have decided to move previous logic from Handler inside ContentService and resolve it on API level.

TODO:

  • Implement feature / fix a bug.
  • Implement tests.
  • Fix new code according to Coding Standards ($ composer fix-cs).
  • Ask for Code Review.

@pspanja
Copy link
Contributor

pspanja commented Apr 24, 2019

This really looks like a hack on top of a system that doesn't support versioning per translation. Because of that it comes with a number of weird edge cases. The first one that came to my mind - what happens when you edit a Content with 2 translations, you change the 1st translation but then proceed to publish the 2nd translation?

It would be easy to implement this on top of the Public API. What is the reason for adding it here, instead of implementing it for example in the date-based publisher? The similar question goes for #2598

@ViniTou
Copy link
Contributor Author

ViniTou commented Apr 25, 2019

@pspanja

what happens when you edit a Content with 2 translations, you change the 1st translation but then proceed to publish the 2nd translation?

You will end with published version identical to that one without changes to 1st translation. Would you as a user expect different behavior?

What is the reason for adding it here, instead of implementing it for example in the date-based publisher?

To have consistent publish action across places.

@pspanja
Copy link
Contributor

pspanja commented Apr 25, 2019

You will end with published version identical to that one without changes to 1st translation. Would you as a user expect different behavior?

Not really, it's what I would expect, it's just that this adds another layer of meaning to the whole thing (publishing drafts/fields/translations). I guess it's a matter of how you approach the whole thing, I tend to think Repository should be made as simple and straightforward as possible, and specific behavior should be implemented on top of it. But it's not really my prerogative anymore, so in any case, thanks for the reply 👍

@ViniTou ViniTou changed the base branch from master to 7.5 April 26, 2019 08:37
eZ/Publish/Core/SignalSlot/ContentService.php Outdated Show resolved Hide resolved
eZ/Publish/Core/Repository/ContentService.php Outdated Show resolved Hide resolved
eZ/Publish/Core/SignalSlot/ContentService.php Outdated Show resolved Hide resolved
eZ/Publish/API/Repository/ContentService.php Outdated Show resolved Hide resolved
eZ/Publish/API/Repository/Tests/ContentServiceTest.php Outdated Show resolved Hide resolved
eZ/Publish/API/Repository/Tests/ContentServiceTest.php Outdated Show resolved Hide resolved
eZ/Publish/API/Repository/Tests/ContentServiceTest.php Outdated Show resolved Hide resolved
eZ/Publish/Core/Repository/ContentService.php Outdated Show resolved Hide resolved
@alongosz alongosz changed the title EZEE-2661: Publish selected languages only, copy the rest from published version EZEE-2661: Allowed translation filtering when publishing version May 17, 2019
@ViniTou ViniTou requested a review from alongosz May 17, 2019 13:52
@micszo micszo self-assigned this May 27, 2019
@lserwatka
Copy link
Member

@ViniTou could you look into this test results?

@ViniTou ViniTou force-pushed the EZEE-2661-copy-selected-languages branch from 9aa7806 to 49811ac Compare May 30, 2019 08:19
Copy link
Member

@micszo micszo left a comment

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

6 participants