{"action":"created","issue":{"url":"https://api.github.com/repos/opendatakit/javarosa/issues/323","repository_url":"https://api.github.com/repos/opendatakit/javarosa","labels_url":"https://api.github.com/repos/opendatakit/javarosa/issues/323/labels{/name}","comments_url":"https://api.github.com/repos/opendatakit/javarosa/issues/323/comments","events_url":"https://api.github.com/repos/opendatakit/javarosa/issues/323/events","html_url":"https://github.com/opendatakit/javarosa/pull/323","id":333515216,"node_id":"MDExOlB1bGxSZXF1ZXN0MTk1NzA4ODcz","number":323,"title":"[WIP] Add support for question-level actions from CommCare","user":{"login":"lognaturel","id":967540,"node_id":"MDQ6VXNlcjk2NzU0MA==","avatar_url":"https://avatars3.githubusercontent.com/u/967540?v=4","gravatar_id":"","url":"https://api.github.com/users/lognaturel","html_url":"https://github.com/lognaturel","followers_url":"https://api.github.com/users/lognaturel/followers","following_url":"https://api.github.com/users/lognaturel/following{/other_user}","gists_url":"https://api.github.com/users/lognaturel/gists{/gist_id}","starred_url":"https://api.github.com/users/lognaturel/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lognaturel/subscriptions","organizations_url":"https://api.github.com/users/lognaturel/orgs","repos_url":"https://api.github.com/users/lognaturel/repos","events_url":"https://api.github.com/users/lognaturel/events{/privacy}","received_events_url":"https://api.github.com/users/lognaturel/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":3,"created_at":"2018-06-19T04:28:03Z","updated_at":"2018-06-26T02:07:16Z","closed_at":null,"author_association":"MEMBER","pull_request":{"url":"https://api.github.com/repos/opendatakit/javarosa/pulls/323","html_url":"https://github.com/opendatakit/javarosa/pull/323","diff_url":"https://github.com/opendatakit/javarosa/pull/323.diff","patch_url":"https://github.com/opendatakit/javarosa/pull/323.patch"},"body":"Closes #227\r\n\r\nFollowing the discussion at https://github.com/opendatakit/javarosa/pull/265#issuecomment-379292630, I pulled in changes from https://github.com/dimagi/commcare-j2me/pull/243. I initially tried to cherry-pick the commits but I found the in-progress commits confusing. Because paths are different and changes like all `Vector`s becoming `List`s have been made, it's hard to get a clean patch and I ended up applying the changes manually.\r\n\r\nThe good news is that this works great with the latest Collect version on a device using the test forms both from Dimagi and from @mdudzinski.\r\n\r\nThe bad news is that I pulled in @mdudzinski's tests from #265 and those are failing. The best fix I can think of at the moment is to go up to a higher level in the test and change the question value with `FormEntryController.answerQuestion` since that's where `triggerActionsFromEvent` is called from. That seems to more closely resemble the code that a client would make.\r\n\r\nThe other thing I'm puzzled by is whether `Action` should be in `CoreModelModule`. Dimagi's code does have it. We have several tests that explicitly `registerPrototypes` for all the classes in `CoreModelModule`s and all of those would fail. If the `Action` base class isn't directly serialized then it seems it doesn't belong there?\r\n\r\nPerhaps the biggest thing we need to decide is whether to go with this more complex approach or with what @mdudzinski had at #227. The big advantage I see to this is that it keeps us closer to Dimagi's fork and lets us pull in follow-on changes such as support for a `setlocation` action.\r\n\r\nCC @dcbriccetti @ggalmazor @yanokwa \r\n\r\nAreas to play close attention to:\r\n- Changes have been made to the code that governs adding repeats. The CommCare branch also implements the `jr:insert` action and I worked around changes related to that. Regression testing around repeats will be important."},"comment":{"url":"https://api.github.com/repos/opendatakit/javarosa/issues/comments/400153795","html_url":"https://github.com/opendatakit/javarosa/pull/323#issuecomment-400153795","issue_url":"https://api.github.com/repos/opendatakit/javarosa/issues/323","id":400153795,"node_id":"MDEyOklzc3VlQ29tbWVudDQwMDE1Mzc5NQ==","user":{"login":"lognaturel","id":967540,"node_id":"MDQ6VXNlcjk2NzU0MA==","avatar_url":"https://avatars3.githubusercontent.com/u/967540?v=4","gravatar_id":"","url":"https://api.github.com/users/lognaturel","html_url":"https://github.com/lognaturel","followers_url":"https://api.github.com/users/lognaturel/followers","following_url":"https://api.github.com/users/lognaturel/following{/other_user}","gists_url":"https://api.github.com/users/lognaturel/gists{/gist_id}","starred_url":"https://api.github.com/users/lognaturel/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lognaturel/subscriptions","organizations_url":"https://api.github.com/users/lognaturel/orgs","repos_url":"https://api.github.com/users/lognaturel/repos","events_url":"https://api.github.com/users/lognaturel/events{/privacy}","received_events_url":"https://api.github.com/users/lognaturel/received_events","type":"User","site_admin":false},"created_at":"2018-06-26T02:07:16Z","updated_at":"2018-06-26T02:07:16Z","author_association":"MEMBER","body":"Going over this in more detail, I how see that @mdudzinski had pulled in CommCare code in cea35910096031be8be1660b0a202537ab50ce3e so really the two PRs have a lot in common. \r\n\r\nThere's something I don't quite understand yet that I need to dive more deeply into. Since `triggerActionsFromEvent` is called from `answerQuestion` which is called each time the form is validated, I would expect the value changed event to be triggered on form load, whenever a value for which the trigger is a dependent value, when the form is finalized, etc. That's not good.\r\n\r\nThat's also the major difference now between this branch and #227. That branch uses data serialization to compare old and new values (kind of hacky but there's no IAnswerData comparison; see https://github.com/opendatakit/javarosa/pull/265/files#diff-96c7e556ff24192dbc4851b9b4410bbfR380) and only triggers the event if the value has actually changed."}}
{"actor":{"display_login":"lognaturel"}}