Adds warning on reserved datanames ("all")

[averissimo]

Pull Request

Fixes #1379

Changes description:

• Add warning on console/UI when checking for datanames
• Add pluralize() function that allows a simplification of code (removing a bunch of if / ifelse statements)
• Adds tests for new functionality
  • in UI
  • in Console

Special ask for reviewer

• Consider if the message could be improved :-)

[github-actions]

Unit Tests Summary

  1 files   25 suites   9m 16s ⏱️
269 tests 265 ✅ 4 💤 0 ❌
524 runs  520 ✅ 4 💤 0 ❌

Results for commit 300b789.

♻️ This comment has been updated with latest results.

[github-actions]

Unit Test Performance Difference

Test Suite	$Status$	Time on main	$±Time$	$±Tests$	$±Skipped$	$±Failures$	$±Errors$
module_teal	💔	$74.75$	$+1.39$	$+2$	$0$	$0$	$0$
shinytest2-module_bookmark_manager	💔	$36.69$	$+1.12$	$0$	$0$	$0$	$0$

Additional test case details

Test Suite	$Status$	Time on main	$±Time$	Test Case
init	👶		$+0.04$	.raw_data_and_all
init	👶		$+0.04$	all
module_teal	👶		$+0.84$	multiple_datanames_with_all_and_.raw_data_
module_teal	👶		$+0.54$	single_dataname_with_all_

Results for commit 7faecfc

♻️ This comment has been updated with latest results.

[github-actions]

Code Coverage Summary

Filename                          Stmts    Miss  Cover    Missing
------------------------------  -------  ------  -------  ----------------------------------------------------------------------------------------------------------------------------------------
R/checkmate.R                        24       0  100.00%
R/dummy_functions.R                  47      11  76.60%   27, 29, 41, 52-59
R/get_rcode_utils.R                  12       0  100.00%
R/include_css_js.R                   22      17  22.73%   12-38, 76-82
R/init.R                             99      42  57.58%   150-159, 161, 173-194, 219-222, 229-235, 238-239, 241
R/landing_popup_module.R             25      25  0.00%    61-87
R/module_bookmark_manager.R         158     127  19.62%   47-68, 88-138, 143-144, 156, 203, 238-315
R/module_data_summary.R             203      37  81.77%   26-54, 68, 78, 238, 269-273
R/module_filter_data.R               64       2  96.88%   22-23
R/module_filter_manager.R           230      57  75.22%   56-62, 73-82, 90-95, 108-112, 117-118, 291-314, 340, 367, 379, 386-387
R/module_init_data.R                 74       0  100.00%
R/module_nested_tabs.R              236      91  61.44%   40-142, 174, 199-201, 320, 360
R/module_snapshot_manager.R         216     146  32.41%   89-95, 104-113, 121-133, 152-153, 170-180, 184-199, 201-208, 215-230, 234-238, 240-246, 249-262, 265-273, 304-318, 321-332, 335-341, 355
R/module_teal_data.R                149      10  93.29%   41-48, 84, 135-136
R/module_teal_lockfile.R            131      44  66.41%   32-36, 44-56, 59-61, 75, 85-87, 99-101, 109-118, 121, 123, 125-126, 160-161
R/module_teal_with_splash.R          12      12  0.00%    22-38
R/module_teal.R                     190      87  54.21%   48-143, 158, 184-185, 217
R/module_transform_data.R            54      32  40.74%   17-52
R/modules.R                         280      72  74.29%   173-177, 232-235, 356-376, 384, 534-540, 553-562, 578-626, 659, 671-679
R/reporter_previewer_module.R        19       2  89.47%   30, 34
R/show_rcode_modal.R                 24      24  0.00%    17-42
R/tdata.R                            14      14  0.00%    19-61
R/teal_data_module-eval_code.R       24       0  100.00%
R/teal_data_module-within.R           7       0  100.00%
R/teal_data_module.R                 58       0  100.00%
R/teal_data_utils.R                  10       0  100.00%
R/teal_reporter.R                    68       6  91.18%   69, 77, 125-126, 129, 146
R/teal_slices-store.R                29       0  100.00%
R/teal_slices.R                      63       0  100.00%
R/TealAppDriver.R                   353     353  0.00%    52-735
R/utils.R                           253      38  84.98%   405-454
R/validate_inputs.R                  32       0  100.00%
R/validations.R                      58      37  36.21%   110-377
R/zzz.R                              15      11  26.67%   4-18
TOTAL                              3253    1297  60.13%

Diff against main

Filename                   Stmts    Miss  Cover
-----------------------  -------  ------  -------
R/module_data_summary.R        0      -2  +0.99%
R/utils.R                    +28      +1  +1.42%
TOTAL                        +28      -1  +0.38%

Results for commit: 300b789

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

[llrs-roche]

When I try with the example code at the issue about .raw_data it doesn't show a warning for .raw_data and I don't see a test for this case.

I assume it was added to check_reserved_datanames because it is what is is for, but when I run the testing apps, I see on my terminal:

[WARN] 2024-11-15 13:09:16.0411 pid:9640 token:[] teal <span>
  <code>all</code>
  is reserved for internal use. Please avoid using it as a dataset name.
</span>

Is this expected? I thought it would only show up on the browser?

[llrs-roche]

Remember to add a test for the .raw_data case. I assume that this won't affect the code on teal.gallery or other apps, but all might be used on wild as is not a reserved word.

[averissimo]

@llrs-roche the .raw_data reserved namespace cannot be tested right now as we currently don't support dot-prefixed as explicit datanames.

Thanks for catching the console issue 👍

[llrs-roche]

I can confirm that the messages on the R console disappeared. Thanks also for simplifying some tests.

I realized that there are two messages on the app about the same issue, see image below. Could this be simplified so that it only appears once or is it by design?

---

Locally I see a new error related to renv, I haven't seen modified anything related to this, so not sure why now it is failing now:

Failure (test-module_teal.R:92:13): creation process is invoked for teal.lockfile.mode = "enabled" and snapshot is copied to teal_app.lock and removed after session ended
file.exists(renv_filename) is not TRUE

`actual`:   FALSE
`expected`: TRUE 

Let's see what happens on the GHA, if it pass I'll approve the PR to be merged

[averissimo]

Good eye @llrs-roche I noticed the same oddity

We need to confirm if that is a bug added in the last few weeks or a feature that groups all errors together at the top.

Discussion moved to: #1409

[llrs-roche]

LGTM