Places survey

[sebastianbarry]

No description provided.

[sebastianbarry]

I found the point where start_ts and end_ts were not being entered into the enketo survey for the trip-level object for trip_user_input/trip confirm survey. I made a case for this where the start_ts and end_ts don't get entered, and made sure they do get entered:

Before

After

As you can see, now the trips get removed from the to-label filter when you've labeled them, and they don't come back after hitting the refresh 🔄 button!

[shankari]

I'm glad you were able to fix that - it will unblock me as well.
However, I don't understand the change. As the comment at the top says, the

        // The trip structure is different between the diary and label screens
        // one has the timestamps in properties and the other does not
        // let's support both so we can label from either screen

If you try to label the trip from the diary, for example, it will have data.properties filled out

That's why I have the line

        let tsObj = _state.opts.trip.data? _state.opts.trip.data.properties : _state.opts.trip;

From your screenshots, it looks like the trip has data with features but no properties.
Is this from the label screen or the diary screen?
I wonder if this is a regression triggered by the changes in https://github.com/e-mission/e-mission-phone/blob/master/www/js/diary/infinite_scroll_list.js#L344

• can you test with a change prior to the diary/label refactor? (e.g. git checkout 82c997cb32b6ca23c31a75f2f78ce643951d1d45) and confirm that the code works even without your changes?
• if so, the fix might be to change Timeline.confirmedTrip2Geojson to actually fill in the properties correctly instead of adding additional hacks. that would also allow us to remove the original hack above (let tsObj = ...)

[sebastianbarry]

My previous screenshots were from the Label screen.

When labeling from the Diary screen, _scope.opts.trip.properties does in fact have start_ts and end_ts, unlike labeling from the Label screen:

I will no do the following:

• can you test with a change prior to the diary/label refactor? (e.g. git checkout 82c997c) and confirm that the code works even without your changes?
• if so, the fix might be to change Timeline.confirmedTrip2Geojson to actually fill in the properties correctly instead of adding additional hacks. that would also allow us to remove the original hack above (let tsObj = ...)

[sebastianbarry]

• can you test with a change prior to the diary/label refactor? (e.g. git checkout 82c997c) and confirm that the code works even without your changes?

The code works in this way:

Old Label Screen:

• _state.opts.trip.data.properties is still undefined (the same behavior as our current commit)
• _state.opts.trip.data? evaluates to false, so _state.opts.trip is used instead of _state.opts.trip.data.properties
• _state.opts.trip contains the start_ts and the stop_ts

Old Diary Screen:

• _state.opts.trip.data.properties is defined and contains start_ts and end_ts
• _state.opts.trip.data? evaluates to true, so _state.opts.trip.data.properties is used

---

That being said, the solution appears to be:

1. Find wherever _state.opts.trip.data is defined, make sure that the start_ts and end_ts are added here.

@shankari you had mentioned:

if so, the fix might be to change Timeline.confirmedTrip2Geojson to actually fill in the properties correctly instead of adding additional hacks. that would also allow us to remove the original hack above (let tsObj = ...)

[shankari]

@sebastianbarry

Given this:

_state.opts.trip.data? evaluates to false, so _state.opts.trip is used instead of _state.opts.trip.data.properties

This is clearly a regression caused by the changes to unify the diary and label screens.

Not sure why this is your proposed solution:

Find wherever _state.opts.trip.data is defined, make sure that the start_ts and end_ts are added here.

instead of my proposal to change Timeline.confirmedTrip2Geojson

Can you indicate why your proposed solution is better?
Do you (and @JGreenlee) understand the cause of the regression?

[JGreenlee]

We can now have the dynamic config checked for updates on launch!

dev-emulator-program.nrel-op.json

{
    "version": 1,
    "autoRefresh": true,
    ...
}

If the above config is loaded into the app (from github) and then it is incremented to version 2, the app will load the new version on its next launch. If autoRefresh is not given, the default behavior is the same.
This will be helpful for development, and potentially for deployers as well

We should now update the docs to explain this
@sebastianbarry Do you want to update the readme you made? Or if you'd rather give me write access to sebastianbarry/e-mission-docs I can do it later

[shankari]

This is looking pretty good to me so far. The two issue that I see are:

• the matching code is not correct
• the HTML file still has a lot of style-based function calls that should be replaced with variables instead.

[shankari]

Only high level issue that is left is to avoid using functions for classes/conditionals etc in the HTML

[shankari]

I don't see any pending issues for the trip code.