41951 Checkbox "Show property status" in unit lists#670
41951 Checkbox "Show property status" in unit lists#670dai-eastgate wants to merge 35 commits intomasterfrom
Conversation
Pull Request Test Coverage Report for Build 10607608374Details
💛 - Coveralls |
fredericalpers
added
UI/UX
andernath
left a comment
andernath
left a comment
There was a problem hiding this comment.
@dai-eastgate I have a question. Please take a look.
| }); | ||
| return $pInputModel; | ||
| } | ||
|
|
There was a problem hiding this comment.
This function already exist in FormModelBuilderDBEstateListSettings
Why would you duplicate it here?
There was a problem hiding this comment.
Sorry for the confusion. The function was indeed duplicated, and I have updated the code to remove it. Could you please review it again? Thank you.
There was a problem hiding this comment.
No problem @dai-eastgate :)
Could you first take a look at the unit tests please?
There was a problem hiding this comment.
@andernath The unit tests have been addressed in the PR at oo-wp-plugin#792. Could you please review it first?
There was a problem hiding this comment.
@dai-eastgate Ops sorry I my fault, missed your comment from yesterday. Of course I will start reviewing fist. :)
|
Steps to install the approved version:
|
|
Steps to install the approved version:
|
|
@dai-eastgate While testing I noticed that the implementation of this feature only applies to the unit lists inside of an master property. As seen in the screenshot below. However, the status is also inherited from the "normal" detail view to the unit detail view. The checkbox must therefore also affect the status banner inherited from the detail view as seen in the second screenshot below. How to setup the unit lists: https://wp-plugin.onoffice.com/en/advanced-features/unit-lists/ Marketing status unit list
Marketing status detail view
|
|
@fredericalpers I will check it |
|
@fredericalpers @andernath I fixed it and during testing, I discovered an issue where the unit detail was not loading the necessary script. I've fixed it in this PR, could you take a look? |
dai-eastgate
force-pushed
the
41951-checkbox-show-property-status-in-unit-lists
branch
from
8e6364f to
02bf0fc
Compare
dai-eastgate
force-pushed
the
41951-checkbox-show-property-status-in-unit-lists
branch
from
02bf0fc to
9982761
Compare
|
Steps to install the approved version:
|
|
@fredericalpers ready for testing |
4 participants
related to #642
changed log:
Update the checkbox to show estate status in unit lists