Mode/Purpose Confirm Survey #572

atton16 · 2019-05-23T11:13:31Z

In this PR we are trying to implement:

Survey display for Mode & Purpose confirmation
Survey display with previous answer restoration (if any)
Dynamic button label display based on previously answered survey on both Mode Button and Purpose Button

At this point, I am struggle at Point (2). It appears that somehow $window.cordova.plugins.BEMUserCache.getAllMessages complicate things up.

If you look at the code in the file js/survey/enketo-survey-services.js line 55-64 never get executes.

Here is the output:

[phonegap] [console.log] EnketoSurvey.init()
[phonegap] [console.log] try to restore answer
[phonegap] [console.log] DEBUG:Answers stored locally[{"trip_properties":{"end_ts":1437582214.517,"start_ts":1437578093.881},"data":"<trip-end-survey_v5 xmlns:jr=\"http://openrosa.org/javarosa\" xmlns:odk=\"http://www.opendatakit.org/xforms\" xmlns:orx=\"http://openrosa.org/xforms\" id=\"trip-end-survey_v5\" version=\"veMfY6eZ2W9qoGhmfzJX8F\"><start>2019-05-23T17:19:43.089+07:00</start><end>2019-05-23T17:19:43.098+07:00</end><group_vs9ga59><trip_purpose_main>1</trip_purpose_main><trip_purpose_sub_category/></group_vs9ga59><group_ee9rk21><travel_mode_main>walk</travel_mode_main><travel_mode_sub_category/><Total_people_in_trip_party>1</Total_people_in_trip_party><Non_household_member_s_on_trip>0</Non_household_member_s_on_trip><Household_member_s_on_trip>1</Household_member_s_on_trip><group_bn3jd79><Vehicle_trip_with_2_Driver_or_passenger/><Vehicle_trip_Parking_location/><Parking_cost/></group_bn3jd79><Rail_payment_method/><Transit_fees_AUD/><Taxi_fees/></group_ee9rk21><__version__>vLX8oPb8xP2pLvxNBagygU</__version__><_version_>vwpCadzPYVsW9XEjxZzwnR</_version_><_version__001>vSuFDQ2DcmpRpKcRfSXReq</_version__001><_version__002>veMfY6eZ2W9qoGhmfzJX8F</_version__002><meta><instanceID>uuid:69bb39ae-6420-471a-afe0-3012e4de23c8</instanceID></meta></trip-end-survey_v5>"}]
[phonegap] [console.log] DEBUG:[object Object]
[phonegap] [console.log] DEBUG:[object Object]
[phonegap] [console.log] DEBUG:found
[phonegap] [console.log] DEBUG:false

At first the problem was worse, when the code was combined in the EnketoSurveyCtrl controller (now resides in the file js/survey/enketo-survey.js) the button in the corresponding template cannot call validateForm function. So my assumption is that $scope get lost in the process of executing $window.cordova.plugins.BEMUserCache.getAllMessages. Now that I have separate the controller and service, I am stuck with the same bizarre problem where some code gets lost and never fired up.

AmplabJenkins · 2019-05-23T11:19:33Z

Can one of the admins verify this patch?

shankari

Few small suggestions based on the current state of the draft.
This does look super promising otherwise...

www/js/survey/enketo-survey-services.js

www/js/survey/enketo-survey.js

atton16 · 2019-05-24T07:18:38Z

In this PR we are trying to implement:

Survey display for Mode & Purpose confirmation

Survey display with previous answer restoration (if any)

Dynamic button label display based on previously answered survey on both Mode Button and Purpose Button

At this point, I am struggle at Point (2). It appears that somehow $window.cordova.plugins.BEMUserCache.getAllMessages complicate things up.

If you look at the code in the file js/survey/enketo-survey-services.js line 55-64 never get executes.

Here is the output:
[phonegap] [console.log] EnketoSurvey.init()
[phonegap] [console.log] try to restore answer
[phonegap] [console.log] DEBUG:Answers stored locally[{"trip_properties":{"end_ts":1437582214.517,"start_ts":1437578093.881},"data":"<trip-end-survey_v5 xmlns:jr=\"http://openrosa.org/javarosa\" xmlns:odk=\"http://www.opendatakit.org/xforms\" xmlns:orx=\"http://openrosa.org/xforms\" id=\"trip-end-survey_v5\" version=\"veMfY6eZ2W9qoGhmfzJX8F\"><start>2019-05-23T17:19:43.089+07:00</start><end>2019-05-23T17:19:43.098+07:00</end><group_vs9ga59><trip_purpose_main>1</trip_purpose_main><trip_purpose_sub_category/></group_vs9ga59><group_ee9rk21><travel_mode_main>walk</travel_mode_main><travel_mode_sub_category/><Total_people_in_trip_party>1</Total_people_in_trip_party><Non_household_member_s_on_trip>0</Non_household_member_s_on_trip><Household_member_s_on_trip>1</Household_member_s_on_trip><group_bn3jd79><Vehicle_trip_with_2_Driver_or_passenger/><Vehicle_trip_Parking_location/><Parking_cost/></group_bn3jd79><Rail_payment_method/><Transit_fees_AUD/><Taxi_fees/></group_ee9rk21><__version__>vLX8oPb8xP2pLvxNBagygU</__version__><_version_>vwpCadzPYVsW9XEjxZzwnR</_version_><_version__001>vSuFDQ2DcmpRpKcRfSXReq</_version__001><_version__002>veMfY6eZ2W9qoGhmfzJX8F</_version__002><meta><instanceID>uuid:69bb39ae-6420-471a-afe0-3012e4de23c8</instanceID></meta></trip-end-survey_v5>"}]
[phonegap] [console.log] DEBUG:[object Object]
[phonegap] [console.log] DEBUG:[object Object]
[phonegap] [console.log] DEBUG:found
[phonegap] [console.log] DEBUG:false
At first the problem was worse, when the code was combined in the EnketoSurveyCtrl controller (now resides in the file js/survey/enketo-survey.js) the button in the corresponding template cannot call validateForm function. So my assumption is that $scope get lost in the process of executing $window.cordova.plugins.BEMUserCache.getAllMessages. Now that I have separate the controller and service, I am stuck with the same bizarre problem where some code gets lost and never fired up.

Fixed over our recent Skype meetings.

shankari

@atton16 great job. I see that you can already read and extract parts of the stored survey results - now you just need to put them into the $scope and display them in the UX. I have a couple of minor comments, all dealing with consistency with the rest of the framework.

Would be great if you could address them so that it is easier for it to take advantage of future improvements to the platform after you are done with this project...

shankari · 2019-05-24T16:30:25Z

www/templates/survey/enketo-survey.html

-                <button id="validate-form" class="btn btn-primary" ng-click="validateForm()" style="width:200px; margin-left: calc(50% - 100px);">Validate</button>
-                <a href="#" class="btn btn-primary next-page disabled" style="width: 200px; margin-left: calc(50% - 100px)">Next</a>
+                <button id="validate-form" class="btn btn-primary" ng-click="validateForm()" style="width:200px; margin-left: calc(50% - 100px);">Save</button>
+                <a href="#survey-paper" class="btn btn-primary next-page disabled" style="width: 200px; margin-left: calc(50% - 100px)">Next</a>


neat! I filed an issue with enekto-core for this (https://github.com/enketo/enketo-core/issues/626), and we've been going back and forth on a good solution. I proposed this as an alternative.

www/js/survey/enketo-survey-services.js

…helper-service

…ormat

atton16 · 2019-05-25T15:01:18Z

@shankari Can you please review this?

shankari · 2019-05-25T15:08:38Z

@atton16 I hate to be pedantic about this, but would it be possible for you to be consistent with the coding conventions for the rest of the project?

I use 4 spaces for indentation, no spaces between function and the next (, " for strings and no space between string components.

Your editor is apparently set with 2 spaces for indentation and spaces between function and (.
But that is an arbitrary choice, and not one that is consistent with the rest of the code. Can you change your settings to match mine?

It is fairly common for open source projects to enforce formatting standards (e.g. https://stackoverflow.com/questions/4714648/enforce-coding-standards-in-all-files-of-a-project-at-build, exercism/cpp#274)

As you can see, reformatting messes up git blame and makes it harder to review.

There's probably something clever we can do with jslint (or rather, prettifier https://stackoverflow.com/a/53636796) to check for this, and to incorporate it into continuous integration, but I don't have time to work on it right now :(

shankari · 2019-05-25T15:11:33Z

I should really put the formatting guidelines into a CONTRIBUTING guide for the future. @atton16 do you have thoughts on where best to put it? I already have a CONTRIBUTING document in e-mission-docs - should I put the guidelines in there, or have a specific one for each repository?

atton16 · 2019-05-25T15:13:57Z

I think best is to use linting tool, I normally use eslint for most of my project.

atton16 · 2019-05-25T15:16:02Z

Actually, I set spaces to 2 on purpose. It makes everything tight especially large project. So it is easier to read complex function as most of the time they are all in the same screen. But I am happy with any. Leave it to you to decide. I can setup lint for you to get it started.

shankari · 2019-05-26T14:11:32Z

@atton16 thank you so much for setting up eslint and fixing all the bugs, but this makes the code even harder to review since the linting changes are overwhelming the real changes.

There are now 71 files that are changed, and I know that the changes related to mode/purpose confirm survey only affect 5-6 at the most.

Is it possible for you to generate a PR that has only the changes required for the mode/purpose confirmation, with no additional whitespace changes?

And then generate a second PR that is basically ONLY whitespace changes and bug fixes from the linting?

Or is that too hard to do now that everything is mixed in here?

shankari · 2019-05-26T15:11:13Z

For example, you could cherry-pick your initial changes and revert the whitespace changes until only semantic changes are left.

atton16 · 2019-05-26T15:28:21Z

I think it is too late at this stage.

shankari · 2019-05-26T15:35:39Z

when I put in my comment about the spacing, I was really hoping for a clean PR to review. At this point, I have no idea what is a real change and what is not, and git blame is all messed up.

shankari · 2019-05-26T15:39:57Z

did you try cherry picking only the first set of semantic changes and removing whitespace changes? How long did it take?

atton16 · 2019-05-26T15:41:43Z

The set of changes before eslint is this commit.

shankari · 2019-05-26T15:45:09Z

that is the last commit, but you have a set, right? You can cherry-pick a set of commits https://git-scm.com/docs/git-cherry-pick

shankari · 2019-05-26T15:47:56Z

so something like

git checkout -b rk-unsw-clean
git cherry-pick rk-unsw..rk-unsw~20

should work

shankari · 2019-06-10T14:51:23Z

superceded by enketo/enketo#302 enketo/enketo#297

atton16 added 2 commits May 23, 2019 18:01

fix: Add support for going back button

a41de7a

fix: Migrate mode/purpose confirm to survey (experimental)

524146f

shankari requested changes May 23, 2019

View reviewed changes

www/js/survey/enketo-survey-services.js Outdated Show resolved Hide resolved

www/js/survey/enketo-survey-services.js Outdated Show resolved Hide resolved

www/js/survey/enketo-survey.js Outdated Show resolved Hide resolved

atton16 added 3 commits May 24, 2019 13:53

fix: Survey answer restoration and some cleanups

4c33183

fix: Survey UI improvement

a380853

fix: Migrate to UnifiedDataLoader

bd84e35

fix: Load survey answers alongwith other unifiedConfirmsResults

af32d70

shankari requested changes May 24, 2019

View reviewed changes

atton16 added 14 commits May 25, 2019 17:09

fix: Update survey files

00a9da8

fix: Update survey files in in the app

99452c4

fix: Separate diary/services.js into diary-helper-service & timeline-…

fb61a9b

…helper-service

fix: Move getUserInputForTrip to trip-confirm-services

82b2518

fix: Get rid of duplicate codes

f56e091

fix: Service naming corrections

b95db80

fix: Make manual/confirm_survey conform to getUserInputForTrip data f…

e8e86a5

…ormat

fix: Bug fixes

70b99d3

fix: Bug fixes

d3b1c72

fix: Refactor

4619aa0

fix: Display survey answer on Add Trip Details button

6eccf8f

fix: Fix bug on production environment

8fca84a

fix: Auto-Refresh buttons on DiaryList when come back from survey page

0e4973b

fix: Update syles

8749905

atton16 marked this pull request as ready for review May 25, 2019 15:00

atton16 added 19 commits May 26, 2019 14:33

fix: Refactor js/control dir

6e659d1

fix: Fix bug while refactoring js/control dir

b565cad

fix(lint): Add L to global var

15424e1

fix(lint): Refactor js/diary & bug fix

341a4f4

fix(lint): Refactor js/incident dir

495be37

fix(lint): Allow console.assert

76ea09b

fix: Refactor js/plugin

6fe2cc5

fix(lint): Add cordova to global var

a45665e

fix: Refactor js/splash dir

c608546

fix: Refactor js/stats

97b731a

fix(lint): Add jQuery to global var

5cc2f89

fix: Refactor js/survey

a93de03

fix: Refactor js/tripconfirm

f7aed06

fix: Refactor www/js

fd74e8c

fix: Refactor

19ef6c9

fix: Bug fix app.js

1fe968c

fix: Refactor survey-resources

3a0ac0e

fix: Refactor bin dir

d20758c

fix: Bug fix

7fa80ce

shankari mentioned this pull request May 26, 2019

Cherry-pick mode and purpose survey changes to make them easier to review #575

Merged

shankari closed this Jun 10, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mode/Purpose Confirm Survey #572

Mode/Purpose Confirm Survey #572

atton16 commented May 23, 2019 •

edited

AmplabJenkins commented May 23, 2019

shankari left a comment

atton16 commented May 24, 2019

shankari left a comment

shankari May 24, 2019

atton16 commented May 25, 2019

shankari commented May 25, 2019

shankari commented May 25, 2019

atton16 commented May 25, 2019

atton16 commented May 25, 2019

shankari commented May 26, 2019

shankari commented May 26, 2019

atton16 commented May 26, 2019

shankari commented May 26, 2019

shankari commented May 26, 2019 •

edited

atton16 commented May 26, 2019

shankari commented May 26, 2019

shankari commented May 26, 2019 •

edited

shankari commented Jun 10, 2019 •

edited

Mode/Purpose Confirm Survey #572

Mode/Purpose Confirm Survey #572

Conversation

atton16 commented May 23, 2019 • edited

AmplabJenkins commented May 23, 2019

shankari left a comment

Choose a reason for hiding this comment

atton16 commented May 24, 2019

shankari left a comment

Choose a reason for hiding this comment

shankari May 24, 2019

Choose a reason for hiding this comment

atton16 commented May 25, 2019

shankari commented May 25, 2019

shankari commented May 25, 2019

atton16 commented May 25, 2019

atton16 commented May 25, 2019

shankari commented May 26, 2019

shankari commented May 26, 2019

atton16 commented May 26, 2019

shankari commented May 26, 2019

shankari commented May 26, 2019 • edited

atton16 commented May 26, 2019

shankari commented May 26, 2019

shankari commented May 26, 2019 • edited

shankari commented Jun 10, 2019 • edited

atton16 commented May 23, 2019 •

edited

shankari commented May 26, 2019 •

edited

shankari commented May 26, 2019 •

edited

shankari commented Jun 10, 2019 •

edited