Add Subbasin NHD+ Table #2794

arottersman · 2018-04-20T21:36:48Z

Overview

Similar to #2781 where we got HUC-12 details — get catchment details and geometries and display them in the new catchment table. Also clean up some map-timing bugs.

Connects #2730
Connects #2791

Demo

Testing Instructions

Pull and bundle
In subbasin view, select a huc-12 and confirm the catchment table is populated
Confirm if you select the same huc-12 the catchment details request doesn't execute again
Toggle between the various views in various states (eg Analyze toggle while viewing catchments, exit attenuated and then view again). Confirm there are no errors in the console and nothing breaks on the second viewing.

Add an endpoint similar to the subbasin huc12s endpoint to fetch the id, area, shape, and streams for a set of catchment ids. GET /mmw/modeling/subbasins/catchments/?catchment_comids=%255B%25224780661%2522%252C%25224780663%2522%252C%25224780665%2522%252C%25224780751%2522%252C%25224782293%2522%252C%25224782295%2522%252C%25224782297%2522%252C%25224782307%2522%255D -- where `catchment_comids` is a urlencoded json array Response: [ { "id": 423312 // comid "shape": { "type": "MultiPolygon", ...}, // catchment geojson "stream": { "type": "MultiLineString", ...}, // stream geojson "area": 4.3111 // area in hectares }, ... ]

rajadain · 2018-04-23T20:51:25Z

Looks like the HUC / Catchments detail table isn't being initialized correctly anymore:

It is initialized correctly on develop / staging:

There may be a missing .bootstrapTable() call somewhere. Will see if I can find it.

arottersman · 2018-04-23T20:57:44Z

Hm, I noticed that intermittently before I moved that bootstrap init call to onRender. I'd wager we still need it onAttach and need to be a bit more clever about where we call it again on re-render.

rajadain

+1 great work. This works really well on the whole.

Sending the list of catchments as a URL encoded JSON array is an interesting choice, but it works so I'm not going to challenge it right now 🚀 🐦 .

Code comments are minor suggestions, all optional. The only thing we should fix before merging is the .bootstrapTable() calls so that they always work.

rajadain · 2018-04-23T21:58:16Z

src/mmw/js/src/modeling/models.js

+
+    fetchCatchmentsIfNeeded: function(comids) {
+        var catchments = this.get('catchments');
+        if (!catchments.isEmpty() || this.fetchCatchmentsPromise) { return; }


Prefer returning an immediately resolved promise rather than nothing:

return $.when();

rajadain · 2018-04-23T21:59:11Z

src/mmw/js/src/modeling/models.js

+        if (!catchments.isEmpty() || this.fetchCatchmentsPromise) { return; }
+
+        var encodedComids = encodeURIComponent(JSON.stringify(comids));
+        this.fetchCatchmentsPromise = catchments.fetch({


Similarly, prefer returning this promise rather than losing the reference. This allows the possibility of usage like:

subbasins.fetchCatchmentsIfNeeded.then(function() { ... });

rajadain · 2018-04-23T22:03:25Z

src/mmw/js/src/modeling/gwlfe/subbasin/views.js

+        if (this.catchmentDetails.isEmpty()) {
+            self.listenToOnce(self.catchmentDetails, 'add', this.setupCatchmentDetails, this);
+        } else {
+            this.setupCatchmentDetails();


Calling render within initialize feels like a mistake ... but I'm not seeing any errors in the console. It is possible that this is the cause of the broken bootstrap table. Consider moving it to a custom function and calling that directly here, and also from within onRender.

arottersman · 2018-04-24T16:10:07Z

Great suggestions re: promises. I've pushed one fixup to address the fetching, and another for the table renders (I believe the issue is fixed now though it doesn't look all that pretty).

Sending the list of catchments as a URL encoded JSON array is an interesting choice, but it works so I'm not going to challenge it right now

Definitely with you on this — I wanted it to stay a GET request like its HUC-12 sibling, and for some reason didn't want to consider how the list should come in. Encoding a JSON array happens easily and predictably so it seems okay for now! 🚀 🐦

rajadain

+1 this works great! The bootstrapTable is rendered correctly every time. I did add comment for a slight adjustment to the promises, but after that this should be ready to merge. Nice work!

rajadain · 2018-04-24T17:04:39Z

src/mmw/js/src/modeling/models.js

@@ -825,14 +825,17 @@ var SubbasinDetailModel = Backbone.Model.extend({

    fetchCatchmentsIfNeeded: function(comids) {
        var catchments = this.get('catchments');
-        if (!catchments.isEmpty() || this.fetchCatchmentsPromise) { return; }
+        if (!catchments.isEmpty() || this.fetchCatchmentsPromise) {
+            return $.when();


Sorry, I missed this earlier, but if this.fetchCatchmentsPromise exists, we should probably return that instead of $.when():

if (!catchments.isEmpty() || this.fetchCatchmentsPromise) { return this.fetchCatchmentsPromise || $.when(); }

Oh, that's my bad. I'll fix before merging.

When entering the HUC-12-level view, fetch the catchment details if they have not already been fetched. This provides us the catchment area to eventually calculate the rates, as well as the geometries.

Add template and view to display catchment loads for an active HUC-12 in subbasin view.

The huc-12 totals table re-renders after the huc12 details change. After every re-render the bootstrap table must be reinitialized.

Fix a bug where if you switched to Analyze or Model views while you were viewing a HUC-12 in subbasin mode, the shapes would not get re-added to the map after coming back to the subbasin view.

Use the helper function to clear the subbasin shapes when exiting subbasin mode instead of clearing the collection. The helper function removes the map listeners, which would otherwise try to fire on removed layers.

The huc12 view should only be rendered if there's an active huc12 to populate it with.

arottersman · 2018-04-24T18:12:22Z

@azavea-bot rebuild

arottersman added the in progress label Apr 20, 2018

arottersman force-pushed the arr/subbasin-catchment-table branch from 4f4fdb1 to a65679a Compare April 20, 2018 21:56

arottersman changed the title ~~wip Arr/subbasin catchment table~~ Add Subbasin NHD+ Table Apr 20, 2018

arottersman requested a review from rajadain April 20, 2018 22:04

arottersman assigned rajadain Apr 20, 2018

rajadain approved these changes Apr 23, 2018

View reviewed changes

rajadain assigned arottersman and unassigned rajadain Apr 23, 2018

rajadain approved these changes Apr 24, 2018

View reviewed changes

Alice Rottersman added 6 commits April 24, 2018 13:49

Fetch Subbasin Catchments If Needed

5591554

When entering the HUC-12-level view, fetch the catchment details if they have not already been fetched. This provides us the catchment area to eventually calculate the rates, as well as the geometries.

Add Subbasin Catchment Table View

d782b2d

Add template and view to display catchment loads for an active HUC-12 in subbasin view.

Initialize Bootstrap Table On Render and On Attach

b5c95bb

The huc-12 totals table re-renders after the huc12 details change. After every re-render the bootstrap table must be reinitialized.

Add Subbasin Shapes Back To Map If In Huc-12 Mode

65af20d

Fix a bug where if you switched to Analyze or Model views while you were viewing a HUC-12 in subbasin mode, the shapes would not get re-added to the map after coming back to the subbasin view.

Fix Exit Subbasin View Shape-Clearing Bug

6b6c32e

Use the helper function to clear the subbasin shapes when exiting subbasin mode instead of clearing the collection. The helper function removes the map listeners, which would otherwise try to fire on removed layers.

Only show huc12 View if there's an active huc12

a1d3403

The huc12 view should only be rendered if there's an active huc12 to populate it with.

arottersman force-pushed the arr/subbasin-catchment-table branch from 92b270b to a1d3403 Compare April 24, 2018 17:49

arottersman merged commit 43c0465 into develop Apr 24, 2018

hectcastro removed the in progress label Apr 24, 2018

arottersman deleted the arr/subbasin-catchment-table branch April 24, 2018 18:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Subbasin NHD+ Table #2794

Add Subbasin NHD+ Table #2794

arottersman commented Apr 20, 2018 •

edited

rajadain commented Apr 23, 2018

arottersman commented Apr 23, 2018

rajadain left a comment

rajadain Apr 23, 2018

rajadain Apr 23, 2018

rajadain Apr 23, 2018

arottersman commented Apr 24, 2018

rajadain left a comment

rajadain Apr 24, 2018

arottersman Apr 24, 2018

arottersman commented Apr 24, 2018

Add Subbasin NHD+ Table #2794

Add Subbasin NHD+ Table #2794

Conversation

arottersman commented Apr 20, 2018 • edited

Overview

Demo

Testing Instructions

rajadain commented Apr 23, 2018

arottersman commented Apr 23, 2018

rajadain left a comment

Choose a reason for hiding this comment

rajadain Apr 23, 2018

Choose a reason for hiding this comment

rajadain Apr 23, 2018

Choose a reason for hiding this comment

rajadain Apr 23, 2018

Choose a reason for hiding this comment

arottersman commented Apr 24, 2018

rajadain left a comment

Choose a reason for hiding this comment

rajadain Apr 24, 2018

Choose a reason for hiding this comment

arottersman Apr 24, 2018

Choose a reason for hiding this comment

arottersman commented Apr 24, 2018

arottersman commented Apr 20, 2018 •

edited