280 locked filter

[chlebowa]

Closes #280

Adds a locked property to FilterState class. A locked filter state cannot be disabled or removed but can still be modified. locked and fixed properties can be set independently of one another.

Changes to FilterState class:

• locked argument is added to constructors and wrappers; the argument receives a logical flag and defaults to FALSE
• a private field locked is added to FilterState to store the property
• $get_state is modified to return the locked value in addition to all other filter state properties
• $set state is modified to interrupt an attempt at disabling a filter state that is locked
• $ui is modified to only display the disable and remove buttons if private$locked is FALSE
• $ui is modified to display an additional icon if private$locked is TRUE

Analogous changes are made to FilterStateExpr class.

The locked argument is also added to filter_var.

Furthermore, FilteredData$filter_panel_disable and FilteredData$filter_panel_disable are modified so that locked filter states ignore the global disable button. To this end, disabling the div containing all filter cards ("filter_active_vars_contents") is removed. Filter cards are now disabled solely by setting their state with disabled = TRUE (which locked states ignore).

Remaining actions:

1. Modify FilterStates$remove_filter_state and FilterStates$clear_filter_states so that locked filter states are omitted.
2. Establish whether the behavior in response to the global disable action is as required. The code in FilteredData$filter_panel_enable/disable has not been removed but commented out and marked with "TODO".
3. The lock icon has been transferred from the fixed property to the locked property. A fixed state thus needs a new icon. burst is used as a placeholder but a suitable replacement must be found.
4. Unit tests must be amended.

[m7pr]

Hey @chlebowa this is really well done and the description of the PR is extremely detailed, hence it is easier for someone to take over. I think I will be able to handle that today. We will only need to decide on the final name (again) and the icon but I guess we can do that on the next refinement

[m7pr]

Hey @chlebowa I tried to move forward with

1. Modify FilterStates$remove_filter_state and FilterStates$clear_filter_states so that locked filter states are omitted.

I think the needed code is there but I'd like your opinion on the logger code. Isn't that too much logging right now?
If you're ok with that I'll proceed with tests/

[chlebowa]

I think the needed code is there but I'd like your opinion on the logger code. Isn't that too much logging right now? If you're ok with that I'll proceed with tests/

It might be a bit too much. I suggest three three log entries: "removing" when the method starts, "aborted" if state is locked, and "removed" when state is removed. Have a look at FilterState$set_state to see what I mean.

[github-actions]

Code Coverage Summary

Filename                      Stmts    Miss  Cover    Missing
--------------------------  -------  ------  -------  -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
R/calls_combine_by.R              8       0  100.00%
R/choices_labeled.R              49      14  71.43%   22, 33, 38, 48-53, 65, 69-73
R/count_labels.R                 98       0  100.00%
R/filter_panel_api.R             23       2  91.30%   89, 101
R/FilteredData-utils.R          117      17  85.47%   103-108, 204, 226-235
R/FilteredData.R                521     208  60.08%   107-110, 309-320, 532-540, 574-576, 590-591, 649-656, 698-747, 762-765, 781, 835-868, 883-885, 889-895, 921-950, 972-974, 978-980, 983-994, 998-1007, 1009-1035, 1085, 1141
R/FilteredDataset-utils.R        20       1  95.00%   131
R/FilteredDataset.R             180      67  62.78%   48, 146, 195-201, 229-286, 326-328
R/FilteredDatasetDefault.R      122       9  92.62%   67, 131, 141, 145, 233-237
R/FilteredDatasetMAE.R          117      29  75.21%   27, 107-112, 164-186
R/FilterPanelAPI.R               23      10  56.52%   75, 93, 111, 126-132
R/FilterState-utils.R           286       1  99.65%   657
R/FilterState.R                 359      55  84.68%   278, 286-287, 289, 368, 420, 637, 691-716, 727-745, 780-785, 794-808
R/FilterStateChoices.R          351     108  69.23%   326-329, 334-340, 353, 421-428, 432-449, 451, 495-506, 516-538, 561-564, 567-570, 581-602, 615-616, 626-630, 632-636
R/FilterStateDate.R             249     155  37.75%   252-258, 271, 320-495
R/FilterStateDatettime.R        360     241  33.06%   289-295, 309, 358-628
R/FilterStateEmpty.R             70      39  44.29%   111, 121-126, 142, 156-205
R/FilterStateExpr.R             122      81  33.61%   155, 158, 186-278, 300, 306-335
R/FilterStateLogical.R          250     168  32.80%   203, 274, 277-490
R/FilterStateRange.R            482     182  62.24%   250, 347-353, 361, 393, 431-432, 515-527, 537-545, 563-573, 575-585, 589-603, 605-613, 631-637, 639-644, 656-692, 709-713, 715-719, 724-728, 730-734, 751-769, 804-808, 818-828
R/FilterStates-utils.R           70       9  87.14%   101, 120, 178-184, 206, 233
R/FilterStates.R                417      47  88.73%   75-79, 188, 322-331, 401-413, 457, 543-547, 630-640, 680, 702
R/FilterStatesDF.R                5       0  100.00%
R/FilterStatesMAE.R              25      16  36.00%   39, 59-76
R/FilterStatesMatrix.R            3       0  100.00%
R/FilterStatesSE.R              194     147  24.23%   34, 99-106, 114-121, 144-288
R/include_css_js.R                5       5  0.00%    12-16
R/teal_slice.R                  222       2  99.10%   278, 402
R/test_utils.R                    1       0  100.00%
R/utils.R                        49       2  95.92%   101-102
R/variable_types.R               48      33  31.25%   41-46, 56, 69-104
R/zzz.R                           6       6  0.00%    3-11
TOTAL                          4852    1654  65.91%

Diff against main

Filename                      Stmts    Miss  Cover
--------------------------  -------  ------  --------
R/calls_combine_by.R             +1       0  +100.00%
R/count_labels.R                +98       0  +100.00%
R/filter_panel_api.R            +23      +2  +91.30%
R/FilteredData-utils.R          +19     -26  +29.35%
R/FilteredData.R                +16     -59  +12.95%
R/FilteredDataset-utils.R       +20      +1  +95.00%
R/FilteredDataset.R              -8     -10  +3.74%
R/FilteredDatasetDefault.R      +16      +1  +0.17%
R/FilteredDatasetMAE.R          -91     -30  +3.58%
R/FilterPanelAPI.R                0      +2  -8.70%
R/FilterState-utils.R          +131     -13  +8.68%
R/FilterState.R                +132     -33  +23.45%
R/FilterStateChoices.R         +182     +12  +26.04%
R/FilterStateDate.R             +84     +47  +3.21%
R/FilterStateDatettime.R       +122     +75  +2.80%
R/FilterStateEmpty.R            +29     +23  -16.69%
R/FilterStateExpr.R            +122     +81  +33.61%
R/FilterStateLogical.R         +112     +89  -9.95%
R/FilterStateRange.R           +257     +53  +19.57%
R/FilterStates-utils.R           +2      +4  -5.50%
R/FilterStates.R               +199     -46  +31.39%
R/FilterStatesDF.R             -225     -29  +12.61%
R/FilterStatesMAE.R            -196     -90  -16.04%
R/FilterStatesMatrix.R         -214    -108  +49.77%
R/FilterStatesSE.R             -215     -54  -26.63%
R/teal_slice.R                 +222      +2  +99.10%
R/test_utils.R                   +1       0  +100.00%
R/utils.R                       +10       0  +1.05%
TOTAL                          +849    -106  +8.89%

Results for commit: a90f003

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

[m7pr]

The unit test that pass have me a little puzzled.

Also, the global disable button does not flip and disable individual disable buttons anymore. It worked properly after #277. If this was caused by this PR, it must be fixed, if it also happens on filter_panel_refactor@main, open a new issue.

This is a Request changes, but I can't select that option, being the author of pull request.

I reverted back 66f86fd
but disabled functionality is gonna go with #300

[m7pr]

@gogonzo @chlebowa there are two aspects left on this PR

• global disable/enable button makes locked filters applicable/usable but disables the option to edit them
• when FilterState$set_state() is applied on a locked=TRUE filter with a disabled parameter it works and sets everything else, besides disabled parameter - so this is as intended as locked=TRUE always overrides disabled to be FALSE

As @gogonzo showed today on #300 we will remove disabled completely so I don't think we should invest energy in troubleshooting this and we should resolve this on #300

[gogonzo]

Looks good. Please revert changes in GH action

[m7pr]

video1696221495.mp4

@gogonzo those GH actions changes that you see is the same issue that @kartikeyakirar had yesterday on other PR.
When I created this PR, GitHub portal created a snapshot of filter_panel_refactor@main branch. My PR since that time is compared to this snapshot and not to the actual state of the filter_panel_refactor@main branch. So all the changes that were added to filter_panel_refactor@main branch are visible as changes on my branch, which is up to date with filter_panel_refactor@main but the snapshot made by GitHub portal on this review is not up to date.

Check out the video above to see how I refresh the PR and make an updated snapshot.
It showed 56 commits and 27 files changed before the refresh, and 45 commits and 20 files after the refresh.
Refresh is setting the target branch to something else, and then getting back to the desired branch.

[github-actions]

Unit Tests Summary

    1 files    28 suites   27s ⏱️
395 tests 394 ✔️ 0 💤 1 ❌
878 runs  877 ✔️ 0 💤 1 ❌

For more details on these failures, see this check.

Results for commit ce9734c.