Skip to content

Don't trigger unload protect when setting lat/long#3587

Merged
melton-jason merged 1 commit into
v7.8.12-prereleasefrom
issue-3584
Jun 9, 2023
Merged

Don't trigger unload protect when setting lat/long#3587
melton-jason merged 1 commit into
v7.8.12-prereleasefrom
issue-3584

Conversation

@melton-jason

@melton-jason melton-jason commented Jun 7, 2023

Copy link
Copy Markdown
Contributor

Fixes #3584

To Test:

  • Create and save a new Locality
    • Refresh the page; the Save button should be disabled
    • Try inputting values in the Latitude and Longitude coordinate boxes and ensure there is no abnormalities in Form behavior
  • Query on Locality and find one (or more) that has very precise Latitude 1 and Longitude 1 values
    • Ensure the Save button is disabled when opening the form
    • Repeat the steps under the (Create and save a new Locality) steps

@maxpatiiuk As shown in the code comments, while not related to #3584, I stumbled across another issue. When rendering the Coordinate components, Specify parses the value (initially a string) to a Coord and then converts the Coord to a float and tries to set the once-string value to the new float.
If the value was precise, there may be a loss of precision during the conversion. While most institutions would likely never need values that precise, data integrity is still data integrity.
I was looking at possible solutions, and am considering Number().toFixed(), because we know the database precision for the field. I'll need to look into this more if it is that important of a problem.

Fixes #3584

The resource.set() function was setting an unload protect when
intially loading lats and longs that were very precise.

The intial value (string), was being set to a number which was
not equal to the inital value.

This led records to need saving even when they were just loaded
(from QueryBuilder or other means)
@melton-jason
melton-jason requested review from a team and maxpatiiuk June 7, 2023 03:28

@bronwyncombs bronwyncombs left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The save button is working appropriately

@maxpatiiuk maxpatiiuk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Feels like 50% of forms bug are in LatLongUi and PartialDateUi
It doesn't help that I didn't write any tests for those 😢

@melton-jason
melton-jason merged commit b7a1e44 into v7.8.12-prerelease Jun 9, 2023
@melton-jason
melton-jason deleted the issue-3584 branch June 9, 2023 15:24
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.

3 participants