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

EZP-30754: Fixed handling always available flag for ContentInfo update #2850

Conversation

alongosz
Copy link
Member

@alongosz alongosz commented Oct 31, 2019

Question Answer
JIRA issue EZP-30754
Bug/Improvement yes
New feature no
Target version 6.13, 7.5, master (8.0@dev)
BC breaks no
Tests pass TBD
Doc needed no

This PR fixes two separate issues, related to handling of the always available flag, which occur during ContentInfo (Content metadata) update.

The first issue is about updating only the always available flag and nothing else, as reported via kaliop-uk/ezmigrationbundle#209 and EZP-30754.
To solve that we need to check if there's anything to update before trying to execute database update.
It's not easy with our own database abstraction layer, therefore this PR refactors \eZ\Publish\Core\Persistence\Legacy\Content\Gateway\DoctrineDatabase::updateContent to rely on \Doctrine\DBAL QueryBuilder for that.

The second issue is about fixing erroneous type casting when setting always available flag, both in Legacy Storage Persistence Content Mapper and in the Content\Gateway\DoctrineDatabase::updateAlwaysAvailableFlag method. The latter mistake caused ContentInfo::alwaysAvailable being always set to true during main Translation update, regardless of the actual state.

Implemented also integration tests covering both updating main translation and always available flag only. Those are the special cases handled by different path of code.

QA

The second issue is reproducible only on the API level unfortunately. Content Info after updating main language code (via ContentMetadataUpdateStruct) has always available flag always set to true.
To see the bug you could use app:test command available on my 1.13-based branch: https://github.com/alongosz/ezplatform/tree/ezp-30754-cmd-for-qa
(observe that alwaysAvailable changes though updateStruct didn't touch it).

TODO:

  • Merge up: add is_hidden column (777d30f#diff-92b3a16009ff423f5d87182e80ea7dcaR304).
  • Refactor Content GW (\eZ\Publish\Core\Persistence\Legacy\Content\Gateway\DoctrineDatabase) updateContent method to rely on \Doctrine\DBAL Query Builder.
  • Add checking if there's anything to update before executing update query in the updateContent method.
  • Fix setting always available flag in the Persistence mapper and updateAlwaysAvailableFlag.
  • Implement tests.
  • See if CI passes.
  • Fix new code according to Coding Standards ($ composer fix-cs).
  • Ask for Code Review.

@alongosz alongosz added the Bug label Oct 31, 2019
@alongosz alongosz force-pushed the ezp-30754-fix-handling-contentinfo-always-available-update branch from 050bc8d to 307d99b Compare November 4, 2019 08:45
@adamwojs
Copy link
Member

adamwojs commented Nov 4, 2019

@alongosz CI failure is caused by code style issues. Is it related to #2857?

@alongosz
Copy link
Member Author

alongosz commented Nov 4, 2019

@alongosz CI failure is caused by code style issues. Is it related to #2857?

Yes.

@alongosz alongosz force-pushed the ezp-30754-fix-handling-contentinfo-always-available-update branch 3 times, most recently from ccc2b42 to 54a1995 Compare November 13, 2019 10:25
@alongosz alongosz force-pushed the ezp-30754-fix-handling-contentinfo-always-available-update branch from 2313d0a to 1f00ac5 Compare November 29, 2019 11:23
@micszo micszo self-assigned this Dec 2, 2019
alongosz added a commit to alongosz/ezpublish-kernel that referenced this pull request Dec 2, 2019
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.

Retested first and second issue on eZ Platform EE v1.13.5 with this branch.
Used the draft PR to test on 2.5.

@micszo micszo removed their assignment Dec 4, 2019
@alongosz alongosz merged commit d8f059e into ezsystems:6.13 Dec 5, 2019
@alongosz alongosz deleted the ezp-30754-fix-handling-contentinfo-always-available-update branch December 5, 2019 11:16
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.

4 participants