frc-frecon / frecon

An API for building scouting apps for FRC competitions
MIT License
3 stars 0 forks source link

Dynamic Routes #61

Closed rye closed 9 years ago

rye commented 9 years ago

Instead of using non-DRY methods and writing a bunch of stuff, we can merely include a method that calls a method within the model to get attributes or relations.

For example, the robots for a given Team are, in a sense, "attributes" of that Team. They're considered to be "related," technically, but they're still treated the same. We can then dynamically route all requests from /teams/:id/records to run the #records method on the found Team object, and spit out the records for that team.

All of this is done automagically.

There may be some nonsense that needs to get added back like Competition name conversion, I'm not sure. But this works fairly well.

rye commented 9 years ago

I kept custom .create methods, and they seem to work. @Sammidysam, can you test this and confirm that everything is okay? You can also test the PSV filtering, which is only done on attribute GET requests and index GET requests.

Sammidysam commented 9 years ago

I'm fine with keeping competition name conversion out. As in, the whole match_number_and_competition_to_match_id method is not as needed. I think Frecon.js would be a better place for that stuff. It was really only created because WeBCa was a mess anyway. We could also get rid of getting teams by number in the URL because of having the nice query stuff and Frecon.js being a thing in the future.

rye commented 9 years ago

Yeah. I think it would be pretty nice to get rid of the actual custom stuff that makes things so easy and then just do queries more generally. API clients can do the gruntwork.

Sammidysam commented 9 years ago

Sounds good. I think all of the magic is in controller.rb? Some of those methods we can simply remove, whereas I think we would also need to tweak a find_model method that would not be needed any more.

rye commented 9 years ago

The Controller.match_number_and_competition_to_match_id method can be deleted, and all references can be refactored out. TeamsController has a custom find_model, which can have the same fate. All of the other controllers have custom .create methods that call super, so maybe we should get rid of those as well. Basically, the thing that we need to do is to clean everything up and make the Controller.create method work for everything. Otherwise, I don't think there's much else to do.

Sammidysam commented 9 years ago

I just removed all of the necessary code. I haven't tested it, but I think stuff should still work. I'll try to test it later today.

rye commented 9 years ago

Sounds good. Keep me posted—I'm currently working on some local upgrades to my private deployment server so I'm unavailable.

rye commented 9 years ago

@Sammidysam, how is testing coming? Would you like me to check stuff?

Sammidysam commented 9 years ago

I started yesterday, then stopped when I found a bug. I haven't gotten going again yet. Whether you test things or not depends on if you are willing to wait for me, as I am slow, or not.

-Sam

Sent using CloudMagic [https://cloudmagic.com/k/d/mailapp?ct=pa&cv=7.0.42&pv=5.1] On Sun, Aug 02, 2015 at 1:13 PM, Kristofer Rye < notifications@github.com [notifications@github.com] > wrote: @Sammidysam [https://github.com/Sammidysam] , how is testing coming? Would you like me to check stuff?

— Reply to this email directly or view it on GitHub [https://github.com/frc-frecon/frecon/pull/61#issuecomment-127046640] .[https://github.com/notifications/beacon/AC7z1j-7BlezD79XkwdhYQ5BwnOCdwTQks5ojkdDgaJpZM4Fd8lm.gif]

rye commented 9 years ago

I just cursorily peeked through most of the routes (by dumping out the routing table and all of the procs associated with each route) and the dynamic parts of this seem to work. Since this is going into better-schema before it goes into master, I don't think it matters all that much if something is a bit broken.

Do I have the :+1: to :shipit:, @Sammidysam?

rye commented 9 years ago

If you had a bug, we can always fix that once you identify it here.

Sammidysam commented 9 years ago

Yeah, go ahead. I've started school now so I'm more of a roadblock than a benefit at this point quite honestly. Don't wait on me.

-Sam

Sent using CloudMagic [https://cloudmagic.com/k/d/mailapp?ct=pa&cv=7.0.42&pv=5.1] On Wed, Aug 12, 2015 at 6:32 PM, Kristofer Rye < notifications@github.com [notifications@github.com] > wrote: If you had a bug, we can always fix that once you identify it here.

— Reply to this email directly or view it on GitHub [https://github.com/frc-frecon/frecon/pull/61#issuecomment-130467559] .[https://github.com/notifications/beacon/AC7z1g86uBfrD10bc2SGYZ-P0juoCxrAks5om8EQgaJpZM4Fd8lm.gif]

rye commented 9 years ago

Okay. Merging. :sparkles: