chore: Added phpunit for the filtering function in Row2Mapper::findAll#2019
chore: Added phpunit for the filtering function in Row2Mapper::findAll#2019AIlkiv wants to merge 1 commit intonextcloud:mainfrom
Conversation
AIlkiv
force-pushed
the
chore/add-unittest-for-view-filters
branch
2 times, most recently
from
26aec92 to
46d29ed
Compare
| /* currently not work (frontend filter settings not support multiple values) | ||
| 'Skills is equal to "Management", "Communication"' => [ | ||
| [[['columnId' => 'skills', 'operator' => 'is-equal', 'value' => ['@selection-id-9', '@selection-id-10']]], | ||
| ['Bob'], // Person with skills "Management" (id: 9) and "Communication" (id: 10) | ||
| 'Filter by skills exactly equal to "Management" and "Communication" - should include 1 rows, exclude 4' | ||
| ],*/ | ||
| /* currently work only frontend | ||
| 'Skills contains "Pyt"' => [ | ||
| [[['columnId' => 'skills', 'operator' => 'contains', 'value' => 'Pyt']]], | ||
| ['Alice', 'Charlie'], // People with skills "Python" (id: 3) | ||
| 'Filter by skills containing "Python" - should include 2 rows, exclude 3' | ||
| ],*/ |
There was a problem hiding this comment.
If these don't work, no need keeping them? Might as well remove?
There was a problem hiding this comment.
I think it makes sense to keep them.
Because we need to synchronize filters with the front end in order to paginate on the server in the future.
There was a problem hiding this comment.
I prefer removing them in the meantime. Who knows when in the future we'll do anything, and it could get messy if we start commenting things like this
| /** | ||
| * Converts selection values to IDs based on selection_options | ||
| */ | ||
| protected function convertSelectionValuesToIds(int $columnId, $value) { |
There was a problem hiding this comment.
I wonder if this still works with #1887. Gonna merge that shortly
There was a problem hiding this comment.
I did a rebase, it works
tests/unit/Db/Row2MapperTest.php
Outdated
| ], | ||
| 'Experience years is greater than 5' => [ | ||
| [[['columnId' => 'experience_years', 'operator' => 'is-greater-than', 'value' => 5]]], | ||
| ['Eve'], // People with experience 8, 7 years |
There was a problem hiding this comment.
Should be this?
| ['Eve'], // People with experience 8, 7 years | |
| ['Eve'], // Person with experience 7 years |
tests/unit/Db/Row2MapperTest.php
Outdated
| ['Alice', 'Eve'], // People with experience 5, 8, 7 years | ||
| 'Filter by experience years greater than or equal to 5 - should include 3 rows, exclude 2' |
There was a problem hiding this comment.
Are we including 3 or 2? We have just Alice and Eve?
| 'score' => 92.0, | ||
| 'status' => 'Inactive', | ||
| 'skills' => ['Management', 'Communication'], | ||
| 'is_available' => '"false"', |
There was a problem hiding this comment.
Is it intentional not to have experience_years for Bob?
tests/unit/Db/Row2MapperTest.php
Outdated
| ['Alice', 'Bob', 'Charlie', 'Diana'], // People with experience 2, 3, 5 years | ||
| 'Filter by experience years lower than or equal to 5 - should include 3 rows, exclude 2' |
tests/unit/Db/Row2MapperTest.php
Outdated
| ['Bob'], // No person with empty experience years | ||
| 'Filter by experience years is empty - should include 1 rows, exclude 4' |
There was a problem hiding this comment.
Oh, so it's intentional for Bob not to have experience? So the comment saying No person is not correct?
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
Signed-off-by: ailkiv <a.ilkiv.ye@gmail.com>
AIlkiv
force-pushed
the
chore/add-unittest-for-view-filters
branch
from
46d29ed to
62f2d6d
Compare
|
@enjeck I did a git rebase and made changes according to the comments. |
|
Is it possible that |
|
@enjeck @blizzz I don't understand what to do with this PR. I can see now that another commit ( e4f0753 ) has been made that overlaps this PR by more than 50%. It's not as simple as rebasing these commits, as they are incompatible. And this PR was only the first part. |
3 participants
These filters are mainly used for View.
In this PR, I covered the following column types with tests:
In the future, I will try to add tests for other column types and check AND/OR operators.