search

gwu-libraries / loggins

django/tastypie app for tracking public computing stats
MIT License
7 stars 2 forks source link

T159 floor #183

Closed kerchner closed 10 years ago

kerchner commented 10 years ago

Rewrote data model and all dependent code. This is a large pull request.

Before: Location had .building ('g', 'e', or 'v') and .floor (integer).

After: New model entities for Building, Floor, Zone. Buildings have Floors, Floors have Zones, and Zones have Locations.

URL patterns have also changed (hopefully for the better). Examples: /gelman/floor/2 - all locations on a floor /gelman/location/E-001 - session history on a location /gelman/offline - offline machines in Gelman /offline - all offline machines Currently there is no view for a Zone.

Data migration: 0004 - adds new entities 0005 - custom data migration. First, this adds GW-Libraries specific Buildings, Floors, and Zones. Then it migrates existing Locations by associating each with the appropriate Zone, by parsing the location naming schema. 0006 - removes obsolete Location fields

kerchner commented 10 years ago

I copied the production database and tested the migration (and branch code). This uncovered a couple of minor issues which are now fixed. The pull request is ready for your review (, @darshanrp ).

darshanrp commented 10 years ago

@kerchner I've reviewed this PR and everything looks fine except for one thing. The 'prepend_urls' method in the api is commented. This method is required since all the Macs record their login/logout events through the api using the hostname. Also, we could add 'zone/building/floor' for filtering locations. Should I go ahead and make the changes?

kerchner commented 10 years ago

Thanks @darshanrp . Please go ahead and make the prepend_urls change. I'm not sure I'm clear about the filter you're proposing for zone/building/floor ?

darshanrp commented 10 years ago

@kerchner I was hoping if the api could filter results on building_name or floor_number etc. I tried out a few things for it, but it doesn't seem to be as simple as adding a line 'zone=ALL' for filtering the LocationResource. Since, having the building_name filter is just a good to have feature and not required currently, I'l uncomment the 'prepend_urls' method and merge this. sounds good?

kerchner commented 10 years ago

Sure. How about write a ticket for the filter issue and we can look into it for the next release.

darshanrp commented 10 years ago

sure