Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

EZP-30147: Fixed moving a subtree with invisible items #2561

Merged
merged 3 commits into from
Mar 14, 2019
Merged

EZP-30147: Fixed moving a subtree with invisible items #2561

merged 3 commits into from
Mar 14, 2019

Conversation

Nattfarinn
Copy link
Contributor

Question Answer
JIRA issue EZP-30147
Bug/Improvement yes
New feature no
Target version 6.7
BC breaks no
Tests pass yes
Doc needed no

TODO:

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

@Nattfarinn Nattfarinn requested a review from alongosz March 14, 2019 09:33
alongosz
alongosz requested changes Mar 14, 2019
View reviewed changes
Copy link
Member

@alongosz alongosz left a comment

Choose a reason for hiding this comment

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

@Nattfarinn Let's work together on improving it as I stated below, so we can have complete fix quickly.

eZ/Publish/Core/Persistence/Legacy/Tests/Content/Location/Gateway/DoctrineDatabaseTest.php Outdated Show resolved Hide resolved
@alongosz
Copy link
Member

Added test via a5b73b7 and removed redundant integration test in unit tests via 8feb090. Let's see if Travis agrees.

m-tyrala
m-tyrala approved these changes Mar 14, 2019
View reviewed changes
Copy link

@m-tyrala m-tyrala left a comment

Choose a reason for hiding this comment

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

Tested on ezplatform 1.7, and ezplatform-ee 1.7
QA approve 🎸

@lserwatka lserwatka merged commit ba665e1 into 6.7 Mar 14, 2019
@lserwatka lserwatka deleted the ezp-30147 branch March 14, 2019 14:09
@lserwatka
Copy link
Member

Could you merge it up?

# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@m-tyrala m-tyrala m-tyrala approved these changes

@alongosz alongosz alongosz approved these changes

@adamwojs adamwojs adamwojs approved these changes

@mikadamczyk mikadamczyk Awaiting requested review from mikadamczyk

@ViniTou ViniTou Awaiting requested review from ViniTou

Assignees
No one assigned
Labels
Bug QA approved Ready for review
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Nattfarinn @alongosz @lserwatka @adamwojs @m-tyrala