Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revise notification behaviour for Match Exactly #4000

Merged
merged 3 commits into from
Jul 31, 2020

Conversation

seadowg
Copy link
Member

@seadowg seadowg commented Jul 31, 2020

Closes #3999. Blocked by #3992.

In addition to the problems described in the issue I also made it so error notifications are cleared/cancelled when a sync succeeds.

What has been done to verify that this works as intended?

New/revised tests and was able to verify manually

Why is this the best possible solution? Were any other approaches considered?

I've left some comments inline.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

Good to just check the issue is fixed and that Match Exactly notifications behave as expected.

Before submitting this PR, please make sure you have:

  • run ./gradlew checkAll and confirmed all checks still pass OR confirm CircleCI build passes and run ./gradlew connectedDebugAndroidTest locally.
  • verified that any code or assets from external sources are properly credited in comments and/or in the about file.
  • verified that any new UI elements use theme colors. UI Components Style guidelines

@seadowg seadowg changed the title Reivse notification behaviour for Match Exactly Revise notification behaviour for Match Exactly Jul 31, 2020
@@ -53,40 +52,14 @@ public void whenMatchExactlyEnabled_clickingFillBlankForm_andClickingRefresh_get
}

@Test
public void whenMatchExactlyEnabled_getsLatestFormsFromServer_automaticallyAndRepeatedly() throws Exception {
Copy link
Member Author

@seadowg seadowg Jul 31, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to revise how the tests are written to drive out the new behaviour rather than just adding more. Along with the changed/new unit tests everything was TDD'd out and I'm pretty sure we didn't lose anything.

@@ -113,7 +113,7 @@ the specific language governing permissions and limitations under the License.
android:configChanges="orientation|screenSize"
android:windowSoftInputMode="stateHidden" />
<activity android:name=".activities.InstanceChooserList" />
<activity android:name=".activities.FillBlankFormActivity" />
<activity android:name=".activities.FillBlankFormActivity" android:launchMode="singleTop"/>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a fan of changes here but at least it's driven out by a test in Espresso

@seadowg seadowg removed the blocked label Jul 31, 2020
@seadowg seadowg marked this pull request as ready for review July 31, 2020 14:40
@seadowg seadowg requested a review from lognaturel July 31, 2020 14:40
@lognaturel lognaturel merged commit 67819a4 into getodk:master Jul 31, 2020
@seadowg seadowg deleted the notifications branch July 31, 2020 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fill Blank Form can appear multiple times on back stack and auth dialog should be displayed immediately
2 participants