-
Notifications
You must be signed in to change notification settings - Fork 0
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
Created schema for flexible and extensible forms #11
Conversation
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.
Mostly looks good to me, just a few minor things
src/app/models/mod.rs
Outdated
is_patient: bool, | ||
caregivers: Vec<ObjectId>, | ||
form_templates: Vec<Form>, | ||
form_filled: Vec<Form>, |
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.
Kinda confused why form_templates
and form_filled
are distinct when instances of filled-out forms are stored as Event::FormSubmitted
on a given Form
instance; presumably this is just out of date?
src/app/models/mod.rs
Outdated
created_by: ObjectId, | ||
created_at: DateTime, | ||
questions: Vec<Question>, | ||
events: Vec<Event>, |
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.
Having Event::FormSubmitted
in a list on a given form rather than storing instances of filled-out forms separately (and querying them directly) feels like it could get a bit awkward to work with on the client?
The alternative would be e.g. have three collections, of User
s, Form
s, and FormResponse
s respectively, or similar, where Form
is a collection of questions and FormResponse
references a given Form
and contains answers
Although now that I say that I realize that it'd also be kind of awkward given that we're working with Mongo rather than a traditional relational DB - although Mongo does still have $lookup
and such. After staring at it for long enough I've concluded that this PR's approach might be the best approach after all 😅
src/app/models/mod.rs
Outdated
} | ||
|
||
// In the template, the answer will be set to the default | ||
// it is when the form is opened |
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.
This comment is out of date given that the answer is no longer stored on FreeFormQuestion
.
src/app/models/mod.rs
Outdated
} | ||
|
||
// In the template, the answer will store the default value | ||
// for when the form is opened |
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.
This comment is out of date given that the answer is no longer stored on SliderQuestion
.
src/app/models/mod.rs
Outdated
// it is when the form is opened | ||
#[derive(Serialize, Deserialize, Clone, Debug, Default)] | ||
pub struct FreeFormQuestion { | ||
id: ObjectId, |
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.
Should be serde-renamed to _id
, assuming that we need a unique id at all. Same goes for SliderQuestion
, MultichoiceQuestion
, and MultichoiceQuestionOption
.
} | ||
|
||
#[derive(Serialize, Deserialize, Clone, Debug)] | ||
pub struct QuestionEdited { |
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.
How will questions being added/removed (so that the total number of questions changes) work?
If a given question is edited in such a way that it has a different semantic meaning (i.e. we just replace it entirely), is there any reason to retain the same id rather than just treating it as a deletion + a creation? Is this intended specifically for cases where questions are edited while keeping the same meaning (e.g. typo corrections, or rephrasing the same general question, etc)?
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 good just should change some small things
src/app/models/mod.rs
Outdated
pub type MultichoiceAnswer = ObjectId; | ||
pub type SliderAnswer = f64; | ||
pub type FreeFormAnswer = String; |
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.
Why is it necessary to retype these basic values? They have no close conflicts.
src/app/models/mod.rs
Outdated
fn _example() { | ||
let _form: Form = Form { | ||
id: Some(ObjectId::new()), | ||
title: String::from("Tremors"), | ||
created_by: ObjectId::new(), | ||
created_at: DateTime::now(), | ||
questions: vec![ | ||
Question::Multichoice(MultichoiceQuestion { | ||
_id: ObjectId::new(), | ||
title: String::from("How many times have you experienced this in the last week?"), | ||
options: vec![MultichoiceQuestionOption { | ||
name: String::from("Once"), | ||
_id: ObjectId::new(), | ||
}], | ||
min_selected: 1, | ||
max_selected: 2, | ||
}), | ||
Question::FreeForm(FreeFormQuestion { | ||
_id: ObjectId::new(), | ||
title: String::from("Is there anything else you would like to add?"), | ||
max_length: 200, | ||
min_length: 0, | ||
}), | ||
], | ||
events: vec![ | ||
Event::QuestionEdited(QuestionEdited { | ||
question_id: ObjectId::new(), | ||
former_question: Question::FreeForm(FreeFormQuestion { | ||
_id: ObjectId::new(), | ||
title: String::from("How are you feeling this week?"), | ||
max_length: 100, | ||
min_length: 10, | ||
}), | ||
new_question: Question::FreeForm(FreeFormQuestion { | ||
_id: ObjectId::new(), | ||
title: String::from("Is there anything else you would like to add?"), | ||
max_length: 200, | ||
min_length: 0, | ||
}), | ||
edited_at: DateTime::now(), | ||
edited_by: ObjectId::new(), | ||
}), | ||
Event::FormSubmitted(FormSubmitted { | ||
answers: vec![ | ||
QuestionAndAnswer::Multichoice( | ||
MultichoiceQuestion { | ||
_id: ObjectId::new(), | ||
title: String::from( | ||
"How many times have you experienced this in the last week?", | ||
), | ||
options: vec![MultichoiceQuestionOption { | ||
name: String::from("Once"), | ||
_id: ObjectId::new(), | ||
}], | ||
min_selected: 1, | ||
max_selected: 2, | ||
}, | ||
ObjectId::new(), | ||
), | ||
QuestionAndAnswer::FreeForm( | ||
FreeFormQuestion { | ||
_id: ObjectId::new(), | ||
title: String::from("Is there anything else you would like to add?"), | ||
max_length: 200, | ||
min_length: 0, | ||
}, | ||
String::from("I wasn't able to press the elevator buttons this morning"), | ||
), | ||
], | ||
submitted_at: DateTime::now(), | ||
submitted_by: ObjectId::new(), | ||
}), | ||
], | ||
}; | ||
} |
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.
Please move to doc string example
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.
lgtm!
* Add DB connection check, example endpoints, and docker compose (#10) * ping DB on start to ensure connection; add example endpoint that reads from DB * address Chris's comments * add docker compose for mongodb * add example endpoint that writes to DB * correctly overwrite existing data with example write endpoint * address review comments * Add http examples for example DB endpoints (#12) * Add http examples for example DB endpoints * address PR comments * Created schema for flexible and extensible forms (#11) * created basic user and form struct * edited and expanded schema for flexibility * Removed unused HashMap and created module for models * resolved issues raised in pull request * created doc comments and added the example to the doc comment * fixed non-public test issue * fixed _id problems * finally fixed id issues and tested * add some more docs (#14) * added dependencies to cargo toml * Add endpoints (#15) * add basic user and form endpoints * remove example endpoints * split endpoints and DTOs into separate files * add User::create and Form::create * generate form question IDs on server side * minor fixes * update docs * clippyyyyy * address review comments * added auth to the backend * rolled back dependencies and moved env vars to config * made changes for linter * Alex/auth (#16) * added dependencies to cargo toml * added auth to the backend * rolled back dependencies and moved env vars to config * made changes for linter * created find and submit form function * fixed form id test issues * added find all endpoint for forms associated to the signed in user --------- Co-authored-by: LeavesOfFall <131100486+LeavesOfFall@users.noreply.github.com> Co-authored-by: Chris Graham <Chrisgraham908@gmail.com>
No description provided.