NREL / floorspace.js

Other
68 stars 36 forks source link

Can't reopen floorplan json #244

Closed shorowit closed 6 years ago

shorowit commented 6 years ago

I created this file less than a week ago but cannot re-open it. residential_sfd_template.json.txt

Here is the error that I get:

Uncaught TypeError: t.edge_ids.map is not a function
    at app.9978801bee1c6916623a.js:5
    at Array.map (<anonymous>)
    at app.9978801bee1c6916623a.js:5
    at Array.map (<anonymous>)
    at t.o (app.9978801bee1c6916623a.js:5)
    at Array.<anonymous> (vendor.7ce1c9b6e493a751a511.js:8)
    at t.L.dispatch (vendor.7ce1c9b6e493a751a511.js:8)
    at t.dispatch (vendor.7ce1c9b6e493a751a511.js:8)
    at t.dispatch (app.9978801bee1c6916623a.js:5)
    at s.importFloorplan (app.9978801bee1c6916623a.js:3)
bgschiller commented 6 years ago

@shorowit @macumber I found the problem. I introduced some bad code in b2f2f9dc0480e03760d99bac3a1b6af0ddd6b8b7, on Nov 8th. I have a fix ready.

Question: How much do we care about the floorplans created during that time? Here are some options, in increasing level of effort:

  1. Ditch those floorplans. They cannot be imported.
  2. Send them to me and I will fix them 'by hand'.
  3. I will create a tool that can convert any of these 'broken' floorplans to the correct format.
  4. I will change the app so that it can correct the 'broken' floorplans. This will continue to be maintained and we will add a test to ensure that floorplans from this period will always work.
macumber commented 6 years ago

Not saying we need it for this case, but the editor should have some code to handle updating old JSON files to new ones. Really this only needs to handle JSON files created at tagged releases. I would suggest that we create and save a JSON file for each tagged release, then add a test showing we can import each of these. Does that sound reasonable? For this particular regression I am not really concerned since it didnt involve a release

bgschiller commented 6 years ago

@macumber totally agree. We actually have a floorplan from 2017-08-31, and we test importing that on every build.

The test that coulda/woulda/shoulda caught this is "Do an export, then try doing an import with that file". I'm working on writing that now.

macumber commented 6 years ago

Cool, what do you think about capturing exports from every release and testing those as well?

bgschiller commented 6 years ago

@macumber yep, that sounds good to me, but difficult to automate. I'll plan on adding them manually as we do releases.

bgschiller commented 6 years ago

@shorowit here's the repaired file: residential_sfd_template.json

shorowit commented 6 years ago

Thanks!