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.

Sign up for GitHub

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

Fix forms not being deletable #6369

Merged
merged 2 commits into from
Aug 27, 2024
Merged

Fix forms not being deletable #6369

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4a2b838
Fix forms not be deletable
seadowg Aug 22, 2024
9d960b8
Clarify entity form delete test
seadowg Aug 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...ndroidTest/java/org/odk/collect/android/feature/instancemanagement/DeleteSavedFormTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class DeleteSavedFormTest {
}

@Test
fun whenFormHasCreatedEntity_doesNotAppearInListToDelete() {
fun whenFinalizedFormHasCreatedALocalEntity_doesNotAppearInListToDelete() {
testDependencies.server.addForm("one-question-entity-registration.xml")

rule.withMatchExactlyProject(testDependencies.server.url)
Expand Down
2 changes: 2 additions & 0 deletions forms/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,6 @@ java {

dependencies {
implementation(Dependencies.kotlin_stdlib)
testImplementation(Dependencies.junit)
testImplementation(Dependencies.hamcrest)
}
2 changes: 1 addition & 1 deletion forms/src/main/java/org/odk/collect/forms/instances/Instance.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public static class Builder {
private String geometry;

private Long dbId;
private boolean canDeleteBeforeSend;
private boolean canDeleteBeforeSend = true;
Copy link
Member

Choose a reason for hiding this comment

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

Is it handled correctly when forms are synced from disk and should be finalized?

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you mean if a form has an entity? We don't currently support local entity creation/updates for synced instances. Otherwise, every Instance should now be deletable.


public Builder() {

Expand Down
14 changes: 14 additions & 0 deletions forms/src/test/java/org/odk/collect/forms/instances/InstanceTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package org.odk.collect.forms.instances

import org.hamcrest.MatcherAssert.assertThat
import org.hamcrest.Matchers.equalTo
import org.junit.Test

class InstanceTest {

@Test
fun `canDeleteBeforeSend is true by default`() {
Copy link
Member

Choose a reason for hiding this comment

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

We have that test whenFormHasCreatedEntity_doesNotAppearInListToDelete:

  1. Please clarify what created entity means. You can add a comment or change the test name to make it clear that it takes place on finalizing.
  2. I think it would be good to have a test in the same class where we just save a form as a draft and test if it can be deleted.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it would be good to have a test in the same class where we just save a form as a draft and test if it can be deleted.

I'd thought about that (I actually wrote that test then deleted it), but I realized we already had a test for finalized forms and felt like adding this unit test was simpler than testing all the possible states that saved forms could be in.

val instance = Instance.Builder().build()
assertThat(instance.canDeleteBeforeSend(), equalTo(true))
}
}