-
Notifications
You must be signed in to change notification settings - Fork 8
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
Add cli register #517
Add cli register #517
Conversation
Visit the preview URL for this PR (updated for commit 58fceba): https://ccv-honeycomb--pr517-add-cli-register-10x1mzbw.web.app (expires Thu, 22 Aug 2024 01:39:15 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 4ace1dcea913a952d2a1af84b94a4421bf36e610 |
This is great! Could you create a new study and add some participants using the CLI and then send a few screenshots here? Just to confirm everything is looking good |
Also, @YUUU23 could you set the base of this to be add-commander? It looks like it'll rebase fairly smoothly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! Things are looking really really good here. It's going to take a little bit to bring the changes from the PRs above it into it but I think that would be worth it here. The prompts themselves look fantastic!
@@ -152,6 +249,10 @@ async function actionPrompt() { | |||
name: "Download data", | |||
value: "download", | |||
}, | |||
{ | |||
name: "Register new participant under study", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can they register to new participants AND new studies or just new participants on an existing study?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should automatically register the new study if the study is not found! I will update this description to make it more clear.
We will not have cases where the study is not found, since it will register the new study if not found. |
add commander.parse() to main function Co-authored-by: Robert Gemma <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you resolve the merge conflicts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Question about async functions though
cli.mjs
Outdated
const getRegisteredfStudyRef = (studyID) => FIRESTORE.collection(REG_STUDY_COL).doc(studyID); | ||
|
||
// Get current registered participant array under the StudyID | ||
const getRegisteredParticipantArr = (studyID) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be an async function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just updated this too!
fix: logical expression to change back to casting to bool for prompt …
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, amazing work Megan !! Thank you !
Add
register
to commander, permitting script to be optionally ran withregister
(i.e.npm run cli register [studyID][partipantID]
, skipping prompt first, second, and third prompt when those relative arguments are providedmain()
:ACTION == "register"
):actionPrompt()
: Add choice for registering new study & participantsstudyIDPrompt()
: Directly setSTUDY_ID
to the input read in and returnparticipantIDPrompt()
: Directly setPARTICIPANT_ID
to the input read in and return, modify prompt to ask for "Enter a new participant:"experimentIDPrompt()
: Skip this prompt, directly returnregister
, calling functionregisterDataFirebase(studyID, participantID)
to register the new study in FirestoreconfirmRegisterPrompt()
: prints out current participant IDs under study if there are any (else print message saying there are none) and ask user to confirm that they want to add the new IDs inAdd new variable
const REG_STUDY_COL = "registered_studies";
for the top-level collection we want to access when registeringregisterDataFirebase(studyID, participantID)
:Helper Functions:
getRegisteredfStudyRef(studyID)
: get reference to the study in"registered_studies"
getRegisteredParticipantArr(studyID)
: return empty array if the current study is not initiated yet; else return array of all participant IDs under given studyregisterNewParticipant(studyID, participantID)
: if there's aregistered_participants
array started under the registered study, add the new participantID to the array and update in FirestoreNow, direct firestore editing instructions outlined here: https://honeycomb.ccv.brown.edu/docs/firebase#registering-studies may be completed through CLI