{"action":"created","issue":{"url":"https://api.github.com/repos/dimagi/commcare-android/issues/1884","repository_url":"https://api.github.com/repos/dimagi/commcare-android","labels_url":"https://api.github.com/repos/dimagi/commcare-android/issues/1884/labels{/name}","comments_url":"https://api.github.com/repos/dimagi/commcare-android/issues/1884/comments","events_url":"https://api.github.com/repos/dimagi/commcare-android/issues/1884/events","html_url":"https://github.com/dimagi/commcare-android/pull/1884","id":273593405,"number":1884,"title":"Don't override hierarchy view back-behavior when coming from form entry","user":{"login":"amstone326","id":3723143,"avatar_url":"https://avatars0.githubusercontent.com/u/3723143?v=4","gravatar_id":"","url":"https://api.github.com/users/amstone326","html_url":"https://github.com/amstone326","followers_url":"https://api.github.com/users/amstone326/followers","following_url":"https://api.github.com/users/amstone326/following{/other_user}","gists_url":"https://api.github.com/users/amstone326/gists{/gist_id}","starred_url":"https://api.github.com/users/amstone326/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/amstone326/subscriptions","organizations_url":"https://api.github.com/users/amstone326/orgs","repos_url":"https://api.github.com/users/amstone326/repos","events_url":"https://api.github.com/users/amstone326/events{/privacy}","received_events_url":"https://api.github.com/users/amstone326/received_events","type":"User","site_admin":false},"labels":[{"id":107669776,"url":"https://api.github.com/repos/dimagi/commcare-android/labels/bug","name":"bug","color":"fc2929","default":true},{"id":107669779,"url":"https://api.github.com/repos/dimagi/commcare-android/labels/minor%20(quick%20review)","name":"minor (quick review)","color":"a2f2f0","default":false},{"id":724786974,"url":"https://api.github.com/repos/dimagi/commcare-android/labels/product/invisible","name":"product/invisible","color":"cfd2d6","default":false},{"id":116252758,"url":"https://api.github.com/repos/dimagi/commcare-android/labels/ready%20for%20review","name":"ready for review","color":"fbca04","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":{"url":"https://api.github.com/repos/dimagi/commcare-android/milestones/23","html_url":"https://github.com/dimagi/commcare-android/milestone/23","labels_url":"https://api.github.com/repos/dimagi/commcare-android/milestones/23/labels","id":2836840,"number":23,"title":"commcare_2.41","description":null,"creator":{"login":"amstone326","id":3723143,"avatar_url":"https://avatars0.githubusercontent.com/u/3723143?v=4","gravatar_id":"","url":"https://api.github.com/users/amstone326","html_url":"https://github.com/amstone326","followers_url":"https://api.github.com/users/amstone326/followers","following_url":"https://api.github.com/users/amstone326/following{/other_user}","gists_url":"https://api.github.com/users/amstone326/gists{/gist_id}","starred_url":"https://api.github.com/users/amstone326/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/amstone326/subscriptions","organizations_url":"https://api.github.com/users/amstone326/orgs","repos_url":"https://api.github.com/users/amstone326/repos","events_url":"https://api.github.com/users/amstone326/events{/privacy}","received_events_url":"https://api.github.com/users/amstone326/received_events","type":"User","site_admin":false},"open_issues":4,"closed_issues":15,"state":"open","created_at":"2017-10-13T21:31:56Z","updated_at":"2017-11-14T21:44:36Z","due_on":null,"closed_at":null},"comments":0,"created_at":"2017-11-13T22:00:40Z","updated_at":"2017-11-14T21:56:34Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/dimagi/commcare-android/pulls/1884","html_url":"https://github.com/dimagi/commcare-android/pull/1884","diff_url":"https://github.com/dimagi/commcare-android/pull/1884.diff","patch_url":"https://github.com/dimagi/commcare-android/pull/1884.patch"},"body":"Confirmed with substantial testing that this fixes https://fabric.io/dimagi/android/apps/org.commcare.dalvik/issues/59d37644be077a4dccac473f?time=last-thirty-days, while maintaining the behavior that was the goal of https://github.com/dimagi/commcare-android/pull/640 (which is what introduced this bug). The reason this PR was causing the crash is as follows: If a user went to the hierarchy view from form entry while on a question that was within any type of group, pressing back would result in calling `goUpLevel()`, which modifies the form index without any provisions for re-setting it afterward. Thus when the user pressed back _again_ and ended up back in `FormEntryActivity`, their form index would be screwed up.\r\n\r\nThe change I made here is that we should only override the back button behavior in `FormHierarchyActivity` if we came from the list of saved/incomplete forms. The reason I solved it this way rather than trying to store and re-set the form index before returning to form entry is that I think this is potentially undesirable behavior to employ when the user has come from form entry regardless. For example, in the example I described above, the user would expect pressing back once to take them back to form entry, since it's irrelevant to them that they happen to have been within a nested group before coming to the hierarchy view. In other words, it doesn't make any sense that the behavior of back in this case would vary depending on what type of question the user was on in form entry beforehand. I think this consideration strongly outweighs the value of having `back` take the user up a level if they do happen to start navigating around within the hierarchy view after coming from form entry."},"comment":{"url":"https://api.github.com/repos/dimagi/commcare-android/issues/comments/344412439","html_url":"https://github.com/dimagi/commcare-android/pull/1884#issuecomment-344412439","issue_url":"https://api.github.com/repos/dimagi/commcare-android/issues/1884","id":344412439,"user":{"login":"ctsims","id":155066,"avatar_url":"https://avatars3.githubusercontent.com/u/155066?v=4","gravatar_id":"","url":"https://api.github.com/users/ctsims","html_url":"https://github.com/ctsims","followers_url":"https://api.github.com/users/ctsims/followers","following_url":"https://api.github.com/users/ctsims/following{/other_user}","gists_url":"https://api.github.com/users/ctsims/gists{/gist_id}","starred_url":"https://api.github.com/users/ctsims/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ctsims/subscriptions","organizations_url":"https://api.github.com/users/ctsims/orgs","repos_url":"https://api.github.com/users/ctsims/repos","events_url":"https://api.github.com/users/ctsims/events{/privacy}","received_events_url":"https://api.github.com/users/ctsims/received_events","type":"User","site_admin":false},"created_at":"2017-11-14T21:56:34Z","updated_at":"2017-11-14T21:56:34Z","author_association":"OWNER","body":"@amstone326 After your latest change, would we still need the behavior where you can't use 'back' to navigate up when coming from a form entry session? It seems likely that your latest change would address that issue structurally without limiting the user from pressing 'back' to navigate up.\r\n\r\nIt's likely that it's still the right behavior in-form (IE: \"One hits \"go to prompt\" sees the screen, doesn't want to be there, then hits 'back' to go to the previous screen and instead navigates internally in form heirarchy), just wanted to double check that we were taking advantage of the new code, if possible."}}
{"actor":{"display_login":"ctsims"}}