AgileVentures / asyncvoter-api

For Voting on Stories and Tickets remotely and asynchronously e.g. planning poker
5 stars 7 forks source link

7 cast vote feature #50

Closed Zsuark closed 8 years ago

Zsuark commented 8 years ago

Feature completed as per current specs.

closes #7

Zsuark commented 8 years ago

Marking this as a WIP until we are clear on the REST API.

tansaku commented 8 years ago

great start here @krauszraphael !

I feel like the notes thing is something we'll definitely want eventually, but is not needed for the MVP - the fault here is not yours at all, but the feature stories which we should have locked down closer to being the absolute minimal MVP

Forgive me if I seem like a crazy person, but I've been so badly burnt by feature creep in other projects that I'm kind of jumpy ... :-) I have this extreme simplicity scalpel where I'm always looking to see how we can do less - hope it's not too irritating a mental tick :-)

As per the discussion in the slack chat

POST /stories/:storyId/votes

and

GET /stories/:storyId/votes/:voteId

sounds like a good way to go for the RESTful routes.

Zsuark commented 8 years ago

@tansaku I agree ... no worries. It has been done now, we can re-couple this to the right spot when needed. I think the proposed REST routes are looking good.

Zsuark commented 8 years ago

For the time being, I implemented the notes as a String. Yes, you're right - we can worry about them later.

joaopapereira commented 8 years ago

I do agreee with some on the REST API point. For now on this story I believe we just need to add the POST part

tansaku commented 8 years ago

@joaopapereira @Zsuark agreed

tansaku commented 8 years ago

@Zsuark so we are agreed then on:

POST /stories/:story_id/votes {size: n}

where n is an integer

And so moving forward we should follow the basic REST URL guidelines here: https://en.wikipedia.org/wiki/Representational_state_transfer#Relationship_between_URL_and_HTTP_methods
and we should nest as appropriate to highlight relations between domain entities, avoiding data in the body or query params (i.e. ?story_id=7) where possible

Zsuark commented 8 years ago

@tansaku we have changed the JSON message a little in refactoring, currently we are using:

POST /stories/:story_id/votes {developer: devId, vote: n}

Zsuark commented 8 years ago

@tansaku ready again for review :grinning:

tansaku commented 8 years ago

interesting it looks like all the changes @joaopereira requested were made, but I can't yet merge - I wonder if we need sign off from @joaopereira

I also note that in the ideal world we might have implemented the feature to view votes first, so that the cucumber test would check that the submitted state was persisted, but I think that's splitting hairs at this point :-)

I know others want to build on this so I'm tempted to just squash and merge ...