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

Fix 1 Square Km Draw Tool #3157

Merged
merged 2 commits into from
Aug 27, 2019
Merged

Fix 1 Square Km Draw Tool #3157

merged 2 commits into from
Aug 27, 2019

Conversation

rajadain
Copy link
Member

@rajadain rajadain commented Aug 27, 2019

Overview

As a likely effect of the Lodash upgrade, the 1 Square Km draw tool had stopped working correctly. The main functionality is extracted into a utility function and a unit test added for it, with sample values taken from production which doesn't have the bug. Once we have our failing test, the utility function is amended to remove Lodash references in favor of a more direct implementation, which is easier to understand and produces correct results. This restores the draw tool functionality.

Connects #3156

Demo

image

Testing Instructions

  • Check out this branch and bundle --debug --tests
  • Go to :8000/ and draw a 1km square box
    • Ensure it works

Checklist

  • All JavaScript tests pass ./scripts/testem.sh

The bit of code that generates a 1 square km box from a
given point is isolated into a utility function so it can
be tested separately. Also add a test that exercises the
code with sample values taken from production (where the
failure does not occur).

The failure is likely due to a subtle change in Lodash
behavior caused by the upgrade in 6ac8212.
This was previously done by a dense sequence of Lodash
methods, some of which were affected by the Lodash upgrade
and had stopped working correctly.

Now we make the final array by hand using direct values,
foregoing any Lodash functionality. This makes the code
easier to read and understand, and the tests pass.
@rajadain rajadain added the WPF Funding Source: William Penn Foundation label Aug 27, 2019
Copy link
Member

@caseycesari caseycesari left a comment

Choose a reason for hiding this comment

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

Nice improvement. Tool is working again and the implementation is much clearer.

@caseycesari caseycesari assigned rajadain and unassigned caseycesari Aug 27, 2019
@rajadain rajadain merged commit 611f582 into release/1.25.0 Aug 27, 2019
@rajadain rajadain deleted the tt/bug-square-km-error branch August 27, 2019 20:53
@rajadain
Copy link
Member Author

Thanks for verifying!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WPF Funding Source: William Penn Foundation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants