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

LX-247 Fix lingering polygons and featureId exception #290

Merged
merged 4 commits into from
Dec 7, 2023

Conversation

rogup
Copy link
Collaborator

@rogup rogup commented Dec 2, 2023

What? Why?

Closes #247

We are not redrawing the polygons properly when switching maps or opening new maps.
This is causing old polygons to linger but not be selectable/editable.
And it means that polygons in the new map throw an error when trying to edit them, since their feature IDs are not registered with mapbox.

I was able to reproduce this on both development and production by following these steps:

  1. Draw a polygon
  2. Open a map containing a different polygon
  3. The first polygon lingers but is not selectable. The other polygon throws an error if you select it then click 'Edit'. (see logs in production since it throws an exception gracefully)

I'm pretty sure this is from all the refactoring work a few months ago (replacing component props with Redux and functionalising the Mapbox component), maybe mine oops. It's pretty serious, it seems basically most polygon functionality is broken...

To fix, I have ensured that we redraw polygons when the current map ID changes. And to cover the case when an unsaved map is replaced with a new map (so map ID remains null), I have introduced a temporary 'unsavedMapUuid' to distinguish between unsaved maps and also trigger a redraw. It feels a bit clanky to have such a variable in the redux store for just this purpose, but I couldn't think of a cleaner way to achieve this without adding a complicated sequence of Redux state changes.

What should we test?

  • Follow the repro steps above

Release notes

I think this should go into the next release so that polygon functionality in prod is fixed ASAP

Deployment notes

Documentation updates

@rogup rogup requested a review from ms0ur1s December 3, 2023 18:13
@rogup
Copy link
Collaborator Author

rogup commented Dec 3, 2023

Hey @ms0ur1s good spot for this bug - please could you give my fix a review?

Also, maybe once it's merged, it would be worth you trying to repro the original scenario that found the bug on staging? I'm pretty sure I didn't follow all the steps completely accurately but ended up hitting the featureId exception by just playing around with some of my own maps.

@ms0ur1s
Copy link
Collaborator

ms0ur1s commented Dec 4, 2023

Nice one @rogup, great fix. Good to get some insight into the causes of the error, as maps and polygons are still new to me.

I'll give this a review now and will defo give a test once it's merged in.

@rogup rogup merged commit 2f58aaf into development Dec 7, 2023
2 checks passed
@rogup
Copy link
Collaborator Author

rogup commented Dec 7, 2023

Thanks for reviewing @ms0ur1s ! I've just merged so it will be ready to test on staging in a few mins.
Also, just to let you know, feel free to merge and move to QA any PRs that I send to you for review, once they're approved

@rogup rogup deleted the lx-247-featureid-error branch December 7, 2023 17:35
@ms0ur1s
Copy link
Collaborator

ms0ur1s commented Dec 8, 2023

NIce one @rogup , I'll give it a test.

Cool, I'll follow that procedure from now on.

@ms0ur1s
Copy link
Collaborator

ms0ur1s commented Dec 11, 2023

@rogup, tested on development branch using three different accounts (two existing and one new), each with multiple saved maps with custom polygons.

  • Switched between each map to check for bleed through of polygons from other maps
  • Edited polygons on each map, checking whether editing was possible and for the appearance of the featureId error noted in featureId error when editing a polygon #247.
  • Successfully created new polygons on existing maps, without errors or bleed through
  • Created a new map, added polygons, changed map, returned to the new map and edited polygons
  • Deleted polygons from existing maps
  • Created new account and re-tested the above

Works perfectly, all tests passed on dev, ready for staging.

@lin-d-hop
Copy link
Contributor

Great work team, spotting and fixing this!
LGTM :)

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.

featureId error when editing a polygon
3 participants