Skip to content

Forms : fix thumbnails mismatch #514

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

Merged

Conversation

kaushikrw
Copy link
Collaborator

@kaushikrw kaushikrw commented Jul 15, 2024

Related to issue: #apollo/781

Description:

Fixes an issue where thumbnails for a FormAttachment are re-used. This PR refactors the thumbnail creation logic to fix the issue.

Summary of changes:

  • Thumbnails are no longer saved to a file. Instead they are loaded directly into the state object FormAttachmentState.
  • This is because the Attachment.id property is not unique when the attachment has just been added and is not yet synced with the service. Hence there is no unique property available to name the thumbnail file.

Pre-merge Checklist

  • a vTest Job for this PR has been run
    • link:
  • Unit and/or integration tests have been added to exercise this PR's logic, and the tests are passing:
    • Yes
    • No

@kaushikrw kaushikrw self-assigned this Jul 15, 2024
@kaushikrw kaushikrw requested a review from sorenoid July 15, 2024 21:15
@kaushikrw kaushikrw marked this pull request as ready for review July 15, 2024 21:15
Copy link
Collaborator

@sorenoid sorenoid left a comment

Choose a reason for hiding this comment

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

Just a question for now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it not possible to inject something unique into the FormAttachmentState at creation time, like a UUID, then use that as the name of the file on disk?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That UUID would need be the same every time to re-fetch the thumbnail on the next launch of the Form. Otherwise, it would end up creating the thumbnail many times as the form is closed and re-opened.

@kaushikrw kaushikrw merged commit 6d3c3e3 into feature-branches/forms Jul 16, 2024
@kaushikrw kaushikrw deleted the kaushik/forms/mismatch_thumbnail_fix branch July 16, 2024 22:58
kaushikrw added a commit that referenced this pull request Jul 18, 2024
* added horizontal scrollbar (#473)

* `Forms` : Add attachment tests (#480)

* `Forms` : Refactor tests (#488)

* Update CorePrototypes.kt (#494)

* `Forms` : handle empty attachments (#498)

* restrict download and viewing attachments of 0 size

* show error along with load

* ignore non-default formattachmentelement (#499)

* `Forms` : Fix strings (#500)

* `Forms` : Micro-app update oauth credentials (#507)

* refactor generating new attachment names (#505)

* `Form` : Attachment theming (#490)

* `Forms` : Consume latest api (#506)

* make thumbnails part of the state object (#514)

* add attachment size restrictions for loading (#518)
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.

2 participants