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

Study Details Configuration Views #1244

Merged
merged 45 commits into from Aug 28, 2023
Merged

Study Details Configuration Views #1244

merged 45 commits into from Aug 28, 2023

Conversation

okaycj
Copy link
Contributor

@okaycj okaycj commented Jul 26, 2023

This PR moves the experiment configuration to its own view.

Closes #1243

Study Edit view:
Screenshot 2023-08-23 at 10 26 02 AM
Screenshot 2023-08-23 at 10 26 06 AM
Screenshot 2023-08-23 at 10 26 51 AM
Screenshot 2023-08-23 at 10 26 55 AM
Screenshot 2023-08-23 at 10 26 59 AM
Screenshot 2023-08-23 at 10 27 04 AM

Study Detail:
Screenshot 2023-08-23 at 10 27 47 AM

EFP Config:
Screenshot 2023-08-23 at 10 28 28 AM
Screenshot 2023-08-23 at 10 28 31 AM

External Config:
Screenshot 2023-08-23 at 10 32 00 AM

@becky-gilbert
Copy link
Contributor

@okaycj thanks for the updated screenshots. This is all looking great to me so far! I'll take a closer look once you've marked it ready for review.

One comment so far: the original Study Edit page has a 'Preview' button in the top right (in line with the 'Study Editor' title). I wonder if it's worth moving or adding that to the new Runner/Protocol page? I think the work flow for many researchers (at least for internal studies) is to edit the JSON protocol/generator code, save, and preview from that page to test their changes.

@mekline
Copy link
Contributor

mekline commented Aug 2, 2023

+1 to adding the preview button back to the study details page!

@mekline
Copy link
Contributor

mekline commented Aug 2, 2023

Okay! Here are some thoughts on the study forms. I also opened an issue for updating the documentation! lookit/lookit-docs#327

Names

Let's call the two views "Study Ad" and "Study Details". Both should have the top title bar with "preview study" link (as mentioned above), going to the study detail view in both cases.

Navigation

For both create & edit study, clicking save on Study Ad should send you to Study Details, rather than back to the experimenter's study view. Study details save should send you back to the Experimenter view. (I think this is more natural/closer match to what researchers are used to, @becky-gilbert @okaycj let me know if you agree!)

Study Ad

Below the title, add a description of what's going on:

"We have split the study forms into two views for easier navigation. On this page, you will provide all the information needed for advertising your study on CHS - images, descriptions, and choices about who the study should be advertised to. You will add the actual links or code for your study on the (Study Details)(link) page"

  • Experiment Runner Type - remove from current position, add to the very bottom.

  • Preview summary - New description (this is an unrelated update to sneak in!)

This is the text (limit 300 characters) that participants will see when browsing studies. Most CHS studies involve a single testing session that a family can complete right now on their own. If your study involves something different, please note this in the description! Use a format something like the following:

"In this (study)/(scheduled video call with a researcher)/(four-session study), your child/baby will..."

"Help us learn about [topic] (...in a live video call with a researcher)/(...in a four-session study)

  • Experiment Type (note new title!): let the description read as follows:

"Choose the type of experiment you are creating - this will change the fields that appear on the Study Details page."

Let the text of the options be:

  • Lookit/Ember Frame Player (Default experiment builder)
  • External Study (Choose this if you are posting a study link rather using an experiment builder)

Study Details

Below the title, add a description of what's going on:

"We have split the study forms into two views for easier navigation. On this page, you will provide the study itself - either a study protocol/study code, or a URL to your external study. You can update eligibility and other recruiting information on the (Study Ad)(link) page"

  • (new!) Experiment type: Maybe put this in a grey box to indicate that this is not editable. "You have created [an External][a Lookit/Ember Frameplayer] study. If you need to change study types, please (create a new study)(link to create study view) instead."

For internal studies, I'd like this to be as similar as possible to the existing form - can we reuse the Protocol Configuration single box + checkbox to get the protocol generator, and existing helptext?

One exception - to "Experiment Runner URL", include the helptext "Leave this value alone unless you are using a custom version of the experiment builder."

For external studies:

Can we turn the Scheduled item into a radio button without messing things up on the back end? I'd like to add a title to this item, "Link Type", with the following options:

Unmoderated (Give participants a link to take the study on their own.)
Scheduled (Schedule participants for one-on-one appointments with a researcher.)

And the rest of the external form as-is!

@okaycj okaycj marked this pull request as ready for review August 8, 2023 15:10
@okaycj okaycj requested a review from bleonar5 August 8, 2023 15:10
Copy link
Contributor

@bleonar5 bleonar5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks good to me, for what it's worth given that I can't run the code yet. The only commit that confused me was "Update version of djlint", which contained the djlint update but also a bunch of miscellaneous formatting changes.

@okaycj okaycj changed the title Exp config Study Details Configuration Views Aug 10, 2023
@okaycj
Copy link
Contributor Author

okaycj commented Aug 10, 2023

@bleonar5

The only commit that confused me was "Update version of djlint", which contained the djlint update but also a bunch of miscellaneous formatting changes.

djlint is a django template linter. This is very much an actively maturing project. As the project moves through its development, it'll add feature that'll change how the templates are formatted. You'll see the sort of changes with other such projects. Black comes to mind.

For this PR, I was working with templates a lot and the version we had installed wasn't working with the mscode plugin. Updating made my life easier.

@becky-gilbert
Copy link
Contributor

@okaycj I started reviewing this and will finish ASAP. But wanted to give you a quick heads up that I've just merged my pipe death PR into develop (#1237), so you might get some merge conflicts when you merge this.

Also, I noticed that we both have study migration files with the same numbered prefix (0091), though obviously the file names are different. Not sure the number itself matters.

Copy link
Contributor

@becky-gilbert becky-gilbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @okaycj! Looks like it was a lot of work to pull these two forms apart. Thanks for the refactoring and cleaning up too!
I tested all of the logic that we talked about (when the study should be rejected and/or require rebuilding vs not) and it all looks great to me!
I added a few comments and suggestions. And I have a few very nit-picky suggestions that I'm putting into a PR for the exp-config branch. Please feel free to ignore/close my PR if you don't want to include my changes. You could also ask me to remove specific commits for anything you don't want.

studies/models.py Show resolved Hide resolved
exp/views/study.py Show resolved Hide resolved
web/views.py Show resolved Hide resolved
studies/forms.py Outdated Show resolved Hide resolved
Copy link
Contributor

@mekline mekline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for these updates! On review, I (apologies!) disagree with a couple of my own wording choices, but nothing structural.

First view: Expand the title to "Study Ad and Recruitment", and change the initial blurb to:

We have split the study forms into two views for easier navigation. On this page, you will provide all the information needed for advertising your study on Children Helping Science - images, descriptions, and choices about who should be recruited for your study. You will add the actual links or code for your study on the next page.

(Note that I've removed the link - we actually don't want people clicking on that form before saving an initial version of their study, right?)

Also, let's add a warning to Experiment Type that this isn't editable, and hopefully clarifies an edge case that a bunch of researchers have been running into recently:

The type of experiment cannot be changed after creating a study, so please review the distinction between experiment builder (internal) studies and external studies, and ask a question on Slack if you need help selecting the right option!

If you are building a study that will happen partially on Lookit (e.g. by using the Lookit consent process) and partially on a link that you provide, choose the experiment builder option.


And for the second view, I neglected to reflect the change to keeping the study type choice on the first view, so some of my description was redundant and will be confusing. Please update to:

We have split the study forms into two views for easier navigation. You can update eligibility and other recruiting information on the (Study Ad)(link) page.

@okaycj okaycj requested a review from mekline August 22, 2023 16:33
@mekline
Copy link
Contributor

mekline commented Aug 24, 2023

Okay, I think I see what you mean about that copy when the first study page is visited in edit mode and the study type can no longer be changed. Let me know if this sounds/looks more natural for both Create Study and Edit Study:


You have selected a [TYPE] study. The study type cannot be changed after creation.

Before saving for the first time, please review the distinction between experiment builder (internal) studies and external studies, and ask a question on Slack if you need help selecting the right option!

@mekline
Copy link
Contributor

mekline commented Aug 24, 2023

Approved other than that copy change though!

@okaycj
Copy link
Contributor Author

okaycj commented Aug 25, 2023

@mekline

I am unsure about the You have selected a [TYPE] study. of the copy. Currently, this text seems like it's unneeded, what the user selected is just above this text.

@mekline
Copy link
Contributor

mekline commented Aug 25, 2023

Fair enough! How about just "NOTE: The study type cannot be changed after creation."

@sonarcloud
Copy link

sonarcloud bot commented Aug 28, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 4 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@okaycj okaycj merged commit 273632d into develop Aug 28, 2023
5 checks passed
@okaycj okaycj deleted the exp-config branch August 28, 2023 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move Experiment Runner config to its own view
4 participants