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

fix isVis based on Field annotation #80

Merged
merged 11 commits into from
Dec 4, 2024
Merged

Conversation

yibeichan
Copy link
Contributor

  1. if "@readonly" is in Field annotation, set isVis: False
  2. if "@CALCTEXTis inField annotation, set isVis: False`, we also update compute list
  3. also changed csv.Reader to pandas
  4. added some tests, passed

TODO:
we may want to update the following (line 305) as well, I need more time to figure out the logic here

elif field_type in COMPUTE_LIST:
    rowData["ui"]["readonlyValue"] = True

@yibeichan yibeichan marked this pull request as ready for review December 4, 2024 01:04
@yibeichan
Copy link
Contributor Author

@satra do you remember what this readonlyValue in UI is for?
Currently, if an item is purely based on calculation, we put it True for readonlyValue.
Here we made isVis false for items have @readonly in redcap Field Annotation (like “session_id”, “started_at”, “completed_at”, “duration”). I'm thinking whether we want to set rowData["ui"]["readonlyValue"] = True for them as well

rowData["ui"]["readonlyValue"] = True

@satra
Copy link
Contributor

satra commented Dec 4, 2024

i think it was in the schema to allow showing computed values but not having them be changed (so isVis=True and readOnly=True. whether the computed value should be showed from redcap could be left to a user later to change. i don't know if there is a field in the redcap table or a similar option in redcap. i think your current approach is fine for @readonly options in redcap.

@satra
Copy link
Contributor

satra commented Dec 4, 2024

and set readOnly to True for them.

@yibeichan
Copy link
Contributor Author

for computed values, isVis will be the computing logic, not exactly True.
for @READONLY from redcap, we will have isVis as False but readOnly=True in ui
sounds good? if yes, then you can merge it

@satra satra added release Ready to go! minor labels Dec 4, 2024
@satra satra merged commit 813005f into ReproNim:main Dec 4, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor release Ready to go!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants