WellDone / strato-old

Strato web portal - device management, configuration, and data visualization for MoMo mobile monitors
MIT License
2 stars 2 forks source link

Start using backbone.js for management lists #82

Closed ChrisLoer closed 10 years ago

ChrisLoer commented 10 years ago

Re-opening against dev branch, including click-handler fix.

ChrisLoer commented 10 years ago

I couldn't reproduce the other errors you saw:

Uncaught TypeError: Cannot read property 'isRef' of null manage:10
Uncaught TypeError: Cannot read property 'toString' of null manage-page.js:122
Uncaught TypeError: Cannot read property 'isRef' of null manage:10

I'm thinking they probably have to do with your data set? Can I run against your box to try to reproduce?

amcgee commented 10 years ago

Turns out the other errors were the result of conflicts between the data model in the database and the one in this PR. My bad for not re-initializing but also you'll need to merge from dev before this PR can be merged anyway.

ChrisLoer commented 10 years ago

OK, I've merged the changes -- it was a pretty heavy collision, and I'm rusty at merging, so I'm a little nervous. In place edit, create, delete, and filter seem to work.

No-op edits cause a 500 in response to the empty PUT, but it doesn't matter much and I think it was like that before. We should move towards making the changes directly in the backbone models and syncing them to the server instead of directly doing PUTS/POSTS, but I opted for a direct port for now.

I'm not sure how the user permission stuff is supposed to work, so that would be a good one to double-check. Oh, also I'm unable to create Monitor Groups, but I assumed that was unimplemented.

amcgee commented 10 years ago

Permissions seem to be working fine, yes Monitor Groups have a disconnect between the data model and manage-page definitions ("owner" is required but not settable) so not being able to create them is expected. You can see the specific error in the response from the POST request (see #38). I'm opening #83 for the no-op PUT request issue, it was existing before and should be mostly harmless.

I still get the isRef error and tracked it down. It happens when a non-required field on an instance is left null. You can reproduce it by creating a monitor but omitting the location field (this also happens automatically when a report is received from an unrecognized monitor on the gateway side). Once a null field is in the database, the isRef error will show up every time you navigate to the manage/monitors page and various things will break (because the exception causes an early exit).

amcgee commented 10 years ago

Other than that one remaining issue I think everything looks good functionally, nice work Chris! I'll take a glance at the code tonight and comment if anything jumps out at me.

ChrisLoer commented 10 years ago

OK, I fixed the isRef error with a null guard in the handlebars template. Also I noticed that you can't edit a filled in field to make it null (you get a 500 from the server).