e-mission / e-mission-docs

Repository for docs and issues. If you need help, please file an issue here. Public conversations are better for open source projects than private email.
https://e-mission.readthedocs.io/en/latest
BSD 3-Clause "New" or "Revised" License
15 stars 34 forks source link

Develop an integrated survey solution for onboarding and potentially each trip #727

Closed shankari closed 2 years ago

shankari commented 2 years ago

To use the behavioral data, we need to have a demographic survey for each user. We have typically used external surveys (Qualtrics/Google Forms) before, but those have the following limitations:

A solution that would address all those issues is to store the survey information in mongodb, just like everything else. But we don't want to create a survey builder. Instead, we will use kobotoolbox to create a survey which we will display using the enketo library.

The UNSW group has already integrated with enketo core in the https://github.com/e-mission/e-mission-phone/tree/rciti branch, so let's start by exploring that approach.

If we can generalize this sufficiently, we can also have a config option to create surveys for each trip instead of using the labeling approach.

shankari commented 2 years ago

Merging directly into master does not result in any merge conflicts https://github.com/e-mission/e-mission-phone/pull/830

We could just merge into this subsidiary branch and revert all the UNSW-specific changes that we don't want.

shankari commented 2 years ago

Let's go with that for now; a few additional commits in the history are not too terrible and at least then we don't need to deal with merge conflicts.

Ok so the new plan is:

shankari commented 2 years ago

reverting individual changes results in merge conflicts, especially if they have been modified by subsequent changes.

$ git revert 15cb98e
Auto-merging www/templates/intro/consent-text.html
CONFLICT (content): Merge conflict in www/templates/intro/consent-text.html
error: could not revert 15cb98ea... Modify the consent text file
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
hint: and commit the result with 'git commit'

Trying to revert a range

$ git revert 15cb98e...26df155
error: commit 26df155ca68844c91fc14f1234c153bd5538ef88 is a merge but no -m option was given.
fatal: revert failed

the help around this is

Usually you cannot revert a merge because you do not know which side of the merge should be considered the mainline. This option specifies the parent number (starting from 1) of the mainline and allows revert to reverse the change relative to the specified parent.

Reverting a merge commit declares that you will never want the tree changes brought in by the merge. As a result, later merges will only bring in tree changes introduced by commits that are not ancestors of the previously reverted merge. This may or may not be what you want.

shankari commented 2 years ago

reverting all the way to the end to avoid partial reverts gives us

kshankar-35069s:phone-rciti-branch kshankar$ git revert 15cb98e...d825fc2
[revert_unsw_specific_changes a9738ff7] Revert "chore: remove unused json"
 1 file changed, 30 insertions(+)
 create mode 100644 www/json/trip_confirm_options.json
Auto-merging www/js/diary/list.js
CONFLICT (content): Merge conflict in www/js/diary/list.js
error: could not revert c5ead07d... patch(Diary): update tripgj label directly with the new answer
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
hint: and commit the result with 'git commit'

one correct revert, and one conflict. Let's try to resolve the conflict and see what happens

shankari commented 2 years ago

After resolving that conflict, moving on to the next revert. Keeps repeating this.

$ git commit
On branch revert_unsw_specific_changes
Revert currently in progress.
  (run "git revert --continue" to continue)
  (use "git revert --skip" to skip this patch)
  (use "git revert --abort" to cancel the revert operation)
shankari commented 2 years ago

Ah that is because of https://stackoverflow.com/a/65638479/4040267 used $ git commit --allow-empty and now there is no pending "stuck" git

shankari commented 2 years ago

Went through and reverted all changes to the best of my ability. Have some weird merge conflicts Now comparing differences between:

to figure out which changes need to be retained

shankari commented 2 years ago

Painstakingly went through, reverted all UNSW-specific changes and re-introduced only the changes needed for this program.

Disposition of changes - bower.json - not reverted - intro.css - reverted - diary.css - reverted - style.css - reverted - en.json - **should edit later** - too much reverting - index.html - too much reverting - re-reverted 28a0b637d1f21a2a2794c81250425af06d78a15f - edited to manually remove the isAnalysed functions (5d4145364c315c78730bf402a1c11e10b34059d1) - general-settings.js - remove survey launch since we no longer use an external survey (9cfa0604959ca4441c6e24682efc43adf43ec2b3) - diary/list.js (4271f284ea1c1700d55c5251e85d98a661d3eb43) - restore to multilabel instead of enketo since we want to enable the dashboard - re-enable the code that restores answers between diary and list views - revert whitespace changes - diary/services.js (83163b7aaf4e3b225e21657b2aaea1b7a3af7bdf) - Change the default options to map to MULTILABEL instead of ENKETO To be consistent with the list view - intro.js keep the new survey HTML a lot of random stuff could have come over as part of this change, including the randomized login, but will introduce that later - metrics.js - rename to berkeley - **should edit later** - launch.js - remove merge conflict - answer.js - enketo-demographics.js - enketo-preview.js - enketo-trip-button.js - service.js - input-matcher.js - no changes - multi-label-ui.js - no changes - survey.js - no changes - emailConfig.json - revert - **should edit later** - trip_confirm.json + enketoSurveyConfig.json - expand config samples and remove overrides - www/manual_lib/enketo/* - unchanged - main-control - add the demographics button - intro.html - add the survey file to the intro - content.html - demographics-button.html - enketo logo - form-base.html - inline.html - modal.html - preview.html - summary-trip-button.html - unchanged - wrapper - added enketo-trip-button with a passthrough recompute delay

Current status:

Diary Label
Screen Shot 2022-05-26 at 12 38 50 PM

Let's go ahead and merge the reverts for now We can address the trip survey buttons in a separate commit.

shankari commented 2 years ago

Fixing remaining minor issues:

If there are no user inputs saved (ever), we get this error.

Screen Shot 2022-05-26 at 5 51 53 PM
while reading confirmed tripsCannot read property 'length' of undefined
TypeError: Cannot read property 'length' of undefined
    at Object.im.getUserInputForTrip (http://localhost/_app_file_/data/user/0/edu.berkeley.eecs.emission.devapp/files/phonegapdevapp/www/js/survey/input-matcher.js:20:23)
    at Object.mls.populateManualInputs (http://localhost/_app_file_/data/user/0/edu.berkeley.eecs.emission.devapp/files/phonegapdevapp/www/js/survey/multilabel/multi-label-ui.js:298:50)
    at http://localhost/_app_file_/data/user/0/edu.berkeley.eecs.emission.devapp/files/phonegapdevapp/www/js/survey/multilabel/multi-label-ui.js:280:17
    at Array.forEach (<anonymous>)
    at Object.mls.populateInputsAndInferences (http://localhost/_app_file_/data/user/0/edu.berkeley.eecs.emission.devapp/files/phonegapdevapp/www/js/survey/multilabel/multi-label-ui.js:279:30)
    at http://localhost/_app_file_/data/user/0/edu.berkeley.eecs.emission.devapp/files/phonegapdevapp/www/js/diary/infinite_scroll_list.js:109:41
    at Array.forEach (<anonymous>)
    at http://localhost/_app_file_/data/user/0/edu.berkeley.eecs.emission.devapp/files/phonegapdevapp/www/js/diary/infinite_scroll_list.js:107:16
shankari commented 2 years ago

Next, buttons are not covering the entire row. During testing, we had width=100%, but we reverted it because that is not true for all trip buttons.

https://github.com/e-mission/e-mission-phone/pull/831/files#diff-fdd1795534648690de8f7d6ba83a002fc2068d076bb80bab0c7555ed8f44bc6dL144

Screen Shot 2022-05-26 at 6 16 16 PM
shankari commented 2 years ago

Adding a text-align style, similar to the multi-label buttons centers it

Screen Shot 2022-05-26 at 6 19 36 PM

and adding a width of 100% makes it take up the entire row. Note that the width needs to be on the button, not the parent DIV to override the hardcoded width in the diary.css

Screen Shot 2022-05-26 at 6 22 40 PM
shankari commented 2 years ago

Remaining issues:

Trip inputs are duplicated And don't show up on the label screen
Screen Shot 2022-05-26 at 6 39 46 PM Screen Shot 2022-05-26 at 6 39 58 PM

Not even for the multi-label

https://user-images.githubusercontent.com/2423263/170623816-92d05e25-2b48-4b1b-b609-8cf5c8b5e743.mov

shankari commented 2 years ago

First issue fixed

Screen Shot 2022-05-26 at 6 49 54 PM

Second issue fixed

https://user-images.githubusercontent.com/2423263/170624159-ada650ce-1c3e-4fad-8104-9fcfad4fc322.mov

Third issue fixed

https://user-images.githubusercontent.com/2423263/170631795-5ca2f330-1626-4709-aa7a-0f63370d5f1f.mov

shankari commented 2 years ago

Note that the related server fix is at: https://github.com/e-mission/e-mission-server/pull/853