Skip to content

187 Add task endpoint for submitting no business#21

Merged
tekin merged 1 commit intodevelopfrom
187-no-business-endpoint
Jul 31, 2018
Merged

187 Add task endpoint for submitting no business#21
tekin merged 1 commit intodevelopfrom
187-no-business-endpoint

Conversation

@tekin
Copy link
Copy Markdown
Contributor

@tekin tekin commented Jul 30, 2018

A submission is required every month, regardless of whether the supplier
had any business for the given framework. This endpoint will allow the
frontend app to report a task as having had no business, so suppliers
can fulfil their obligation to submit something.

@tekin tekin changed the title Add task endpoint for submitting no business 187 Add task endpoint for submitting no business Jul 30, 2018
Copy link
Copy Markdown
Contributor

@leeky leeky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of comments, but otherwise looks good! :shipit:

Comment thread config/routes.rb
resources :tasks, only: %i[index show create update] do
member do
post 'complete', to: 'tasks#complete'
post :no_business
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth changing the similar complete route above? We have two styles of saying the same thing very close to each other.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, will change it.

Comment thread app/controllers/v1/tasks_controller.rb Outdated

def no_business
task = Task.find(params[:id])
submission = task.file_no_business!
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels odd - it reads like you're setting a Submission to the result of an action on Task.

I was considering adding a last_submission scope on a Task (and exposing it via the API for that linking to a submission from a task story). You could then do:

task = Task.find(params[:id])
task = task.file_no_business!
submission = task.last_submission

Another option might be to rename the file_no_business! method.

But then again, I'm not sure there's a better solution at the moment!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I struggled with this also. let's look at it when you get in, see if we can find a neater way.

@tekin tekin force-pushed the 187-no-business-endpoint branch from 56b6b15 to 4458e46 Compare July 31, 2018 09:07
A submission is required every month, regardless of whether the supplier
had any business for the given framework. This endpoint will allow the
frontend app to report a task as having had no business, so suppliers
can fulfil their obligation to submit something.
@tekin tekin force-pushed the 187-no-business-endpoint branch from 4458e46 to f46dd46 Compare July 31, 2018 09:39
def no_business
task = Task.find(params[:id])
task.file_no_business!
submission = task.latest_submission
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels much better to me.

@tekin tekin merged commit 92178cd into develop Jul 31, 2018
@tekin tekin deleted the 187-no-business-endpoint branch July 31, 2018 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants