search

NYCPlanning / labs-applicantmaps

5 stars 5 forks source link

Address wizard issues (#538) #556

Closed allthesignals closed 5 years ago

allthesignals commented 5 years ago

Closes #538 by having the back button correctly route users based on project state. Removes some implicit mutations when adding things from the dashboard.

codecov[bot] commented 5 years ago

Codecov Report

Merging #556 into develop will decrease coverage by 1.41%. The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #556      +/-   ##
===========================================
- Coverage    86.89%   85.48%   -1.42%     
===========================================
  Files           74       74              
  Lines         1053     1054       +1     
===========================================
- Hits           915      901      -14     
- Misses         138      153      +15
Impacted Files Coverage Δ
...ts/project-geometries/types/commercial-overlays.js 100% <ø> (ø) :arrow_up:
app/components/map-form.js 95.83% <100%> (-2.07%) :arrow_down:
app/controllers/application.js 0% <0%> (-100%) :arrow_down:
app/components/search-handler.js 70% <0%> (-20%) :arrow_down:
app/components/project-geometries/modes/lots.js 93.87% <0%> (-6.13%) :arrow_down:
app/components/mapbox-gl-draw.js 77.41% <0%> (-3.23%) :arrow_down:
app/components/project-geometries/modes/draw.js 85.26% <0%> (-3.16%) :arrow_down:
app/models/geometric-property.js 97.61% <0%> (+2.38%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 5bbc4e1...13fc8ec. Read the comment docs.

andycochran commented 5 years ago
ghost commented 5 years ago

confused about the diff I'm seeing ... seems to include changes that were from your previously merged PR.

also attach this to the approp ticket (can do at bottom of this comment thread)

also, don't forget to clean up ur branch history before merging! develop is looking mucky right now

allthesignals commented 5 years ago

Refreshing a project URL where the wizard has been completed should not show you the wizard.

@andycochran Did you see my comments? You can't refresh. The fix for that is a change to the API which is linked in the issue. My test for that proves it works if the data being returned from the API is correct.

@julialucyhogan This is happening because I branched this off a previous now-merged PR that I squashed. I'll go ahead and fix this so you can review properly.