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

Add "Updated at" subtext for forms that get updated attachments #5386

Merged
merged 11 commits into from
Jan 19, 2023

Conversation

grzesiek2010
Copy link
Member

@grzesiek2010 grzesiek2010 commented Dec 16, 2022

Closes #5384

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

I've verified the fix manually and added automated tests.

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

Not much to describe here... I've just updated the current implementation to display proper subtext.

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?

As described in the issue a proper subtext should be displayed in the list of forms (in both the list of forms to open and the list of forms to delete). Please test both lists to make sure it works as expected. I can't see any risk in other parts of the app. The description in the issue lists possible cases very well so please read.

Do we need any specific form for testing your changes? If so, please attach one.

Any form with attachments.

Does this change require updates to documentation? If so, please file an issue here and include the link below.

No.

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

@grzesiek2010 grzesiek2010 changed the title Collect 5384 Add "Updated at" subtext for forms that get updated attachments Dec 19, 2022
@grzesiek2010 grzesiek2010 marked this pull request as ready for review December 19, 2022 14:54
@grzesiek2010 grzesiek2010 added the high priority Should be looked at before other PRs/issues label Dec 19, 2022
@srujner
Copy link

srujner commented Jan 18, 2023

We have found one strange behavior related to the sorting.
The problem is with "Sort by: Date, newest first/oldest first"
It looks like Collect ignores the "Updated on" time. At the screenshot below those two marked forms should be at the top of the list. Should we create a separate issue for that?

Screenshot_20230118-095421~2

cc: @lognaturel

@grzesiek2010
Copy link
Member Author

This is something we didn't consider... I agree that it looks a bit strange and probably should be fixed.
What do you think @lognaturel @seadowg?

@seadowg
Copy link
Member

seadowg commented Jan 18, 2023

I think it'd make sense to use the "updated at" value if present for the sort so that the two marked forms in @srujner's example would appear at the top of the list. We can definitely merge (and I'd argue release) this as-is and write a new issue for updating sort, but happy to let you do it as part of this PR if you'd prefer to just get it done @grzesiek2010. Up to you!

@grzesiek2010
Copy link
Member Author

Ok let's have a separate issue for that.

@seadowg
Copy link
Member

seadowg commented Jan 19, 2023

Great. Go ahead and continue testing then @getodk/testers.

@dbemke
Copy link

dbemke commented Jan 19, 2023

Tested with Success!

Verified on Android 10

Verified Cases:

  • changing form attachments in Central and Aggregate
  • changes in forms using linked datasets
  • adding a new version of a form
  • update mode: match exactly, manual, previously downloaded forms only
  • in match exactly update mode refreshing the list manually and automatically
  • hide old form versions enabled/disabled
  • checking the subtext in Fill Blank Form, Get Blank Form, Delete Blank Forms
  • checking the subtext when changed from "added” to "updated” and from "updated” to "added”
  • sorting lists of forms
  • RTL
  • light/ dark mode

@srujner
Copy link

srujner commented Jan 19, 2023

Tested with Success!

Verified on Android 13

Found issues will be created in a separate issue
#5413