timbunce / WebAPI-DBIC

A composable RESTful JSON API to DBIx::Class schemas using roles and Web::Machine. PLEASE NOTE This module is no longer under active development. If you're interested in helping to develop or maintain it please fork it.
26 stars 10 forks source link

Feature/join #35

Open ungrim97 opened 9 years ago

ungrim97 commented 9 years ago

Ad support for DBIC JOIN command from URL Param. #34

coveralls commented 9 years ago

Coverage Status

Coverage increased (+2.0%) to 88.68% when pulling 396b9b8bf0f46bfc13657f1c666ac2299183b039 on feature/join into 840d9e7bf275d0f1fd44443fa595566e31dbdb2a on master.

coveralls commented 9 years ago

Coverage Status

Coverage increased (+2.0%) to 88.68% when pulling 396b9b8bf0f46bfc13657f1c666ac2299183b039 on feature/join into 840d9e7bf275d0f1fd44443fa595566e31dbdb2a on master.

coveralls commented 9 years ago

Coverage Status

Coverage increased (+2.04%) to 88.72% when pulling 396b9b8bf0f46bfc13657f1c666ac2299183b039 on feature/join into 840d9e7bf275d0f1fd44443fa595566e31dbdb2a on master.

timbunce commented 9 years ago

For the record I'm uncomfortable about a few aspects of this:

More fundamentally though, I'm unclear about what the semantics of join=... should be.

Given GET /cd?join=artist&artist.name=Random+Boy+Band the join seems redundant because it's implied by the artist. prefix of artist.name=... Similarly for cds in GET /artist/1?join=cds&order=cds.cdid

castaway commented 9 years ago

You'd prefer thay any required joining be infered by use of the relation names? can't say I care either way really..

What happens if there are multiple ways to join table A to B though? (if A and B are several levels apart..) Lets say for (almost realy) argument: /user?pantry.name=fred .. where user has_many pantries belongs_to pantry.. AND user belongs_to family has_many pantries belongs_to pantry ... how did you know which chain I meant?

ungrim97 commented 9 years ago

The syntax for join should be identical to that of prefetch. Prefetch just effectively adds a columns arg to force returning of the related resources.

RE: Cuddling else. That fine though its probably worth having a wider discussion on code standards

ungrim97 commented 9 years ago

The removale of the _pre_update_resource_method should have been seperate to this work....Woops. Basically we seemed to be providing a pre update hook when the resource can just use method modifiers to achieve the same thing.

timbunce commented 9 years ago

@ungrim97 re _pre_update_resource_method I don't think method modifiers can be used to achieve the same thing. In apps that support multiple media types they'll be multiple method modifiers that are unaware of each other or which should be 'active' for the media type of the current request. I think this is another aspect of the need for abstracting adaptors and serializers. Anyway, off-topic for this PR. I'll comment on the join param later today - gott'a go now.

timbunce commented 9 years ago

@castaway,

What happens if there are multiple ways to join table A to B though? (if A and B are several levels apart..) Lets say for (almost realy) argument: /user?pantry.name=fred .. where user has_many pantries belongs_to pantry.. AND user belongs_to family has_many pantries belongs_to pantry ... how did you know which chain I meant?

because the 'pantry' in pantry.name=... is the name of the relation to use. (Best to avoid 'table' in discussions).

Note that I have relatively narrow actual experience with DBIC and WAPID so I could easily be going down blind alleys and barking up wrong trees. So do push back, ideally with examples, if I don't seem to be talking sense.