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

P#50179 Show or hide reference estate in estate lis #158

Conversation

hoangdoegs
Copy link
Contributor

Recreate pull request

@hoangdoegs hoangdoegs requested a review from jayay December 29, 2021 07:16
@coveralls
Copy link

coveralls commented Dec 29, 2021

Pull Request Test Coverage Report for Build 1844949272

  • 37 of 37 (100.0%) changed or added relevant lines in 5 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.08%) to 78.257%

Totals Coverage Status
Change from base Build 1805367272: 0.08%
Covered Lines: 6018
Relevant Lines: 7690

💛 - Coveralls

@michaelarnold-dev
Copy link
Contributor

@nglelinh Test in https://wpptest.onofficeweb.com/ with version 2.20.12-1-gff712798:

When trying to save a property list, I always get the error message: "An error occurred while saving the view. Please make sure that no other view with this name exists, even if it has a different type."

->> Property lists should be able to be saved. There is no view with the same name here.

@hoangdoegs
Copy link
Contributor Author

@jayay I checked it.
Because db_version hasn't been updated yet, the query add column has not been run.
Please check again

@jayay
Copy link
Contributor

jayay commented Feb 8, 2022

@hoangdoegs DatabaseChanges::getQueryAddColumnShowReferenceEstateTableEstateView adds an entire new CREATE TABLE. This shouldn't happen. The new columns should be added in the method getCreateQueryListviews and dbDelta($this->getCreateQueryListviews()) should be called inside the if ($dbversion == 21) { block.

@michaelarnold-dev
Copy link
Contributor

@nglelinh Test with version 2.21.6-4-gc171e430, https://wpptest.onofficeweb.com/

The errors in #2754903 and #2754937 are not fixed for me.

1.) Requirement: By default the checkbox should be selected. This way the change remains downward compatible.
--> In existing and new property lists the checkbox is not selected.

2.) Requirement: If the new checkbox in backend is not selected: The reference properties are filtered out from the list view and no longer be displayed.
->> Removing the checkbox has no effect on me. I.e. reference properties are displayed in the frondend although the checkbox is unchecked.

@hoangdoegs
Copy link
Contributor Author

@michaelarnold-dev Could you please check this PR?

plugin/EstateList.php Outdated Show resolved Hide resolved
@michaelarnold-dev michaelarnold-dev merged commit c613168 into master Feb 17, 2022
@jayay jayay deleted the 15162-show-or-hide-reference-estate-in-estate-list-fix branch March 8, 2022 12:26
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.

6 participants