Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement the /movies endpoint #27

Closed
KenavR opened this issue Oct 16, 2017 · 5 comments
Closed

Implement the /movies endpoint #27

KenavR opened this issue Oct 16, 2017 · 5 comments

Comments

@KenavR
Copy link
Member

KenavR commented Oct 16, 2017

Dependencies:

Generally /movies (GET) returns the entire list of movies from the database.

@KenavR
Copy link
Member Author

KenavR commented Oct 20, 2017

Great. Sadly I didn't have time so far to define the endpoint or the model, but I will add it today or tomorrow. Anyhow, I think you could implement the basic structure now, with just a movie name.

I would just make one suggestion, move the logic that loads the movies from the database into a "MovieService" or something, so people can use it for other endpoints (e.g. /browse) and we are able to maybe extend it with filters, limits or offset in the future.

Edit: Weirdly I can't assign you the issue. I am not sure if the reason for that is, that you are not part of the organisation, but consider it assigned to you.

@KenavR KenavR assigned KenavR and unassigned KenavR Oct 20, 2017
@AndrewMusgrave
Copy link
Contributor

Was this assigned to anyone?

@KenavR
Copy link
Member Author

KenavR commented Nov 7, 2017

No, it wasn't, the person deleted their comments and said they would come back later. If you'd like to work on it, it would be appreciated. I won't be able to update the models today. I am at work until 9 pm and I have the first part of a paper due today. I promise I will update them tomorrow.

@AndrewMusgrave
Copy link
Contributor

Sure I'll start it now.

I would just make one suggestion, move the logic that loads the movies from the database into a "MovieService" or something, so people can use it for other endpoints (e.g. /browse) and we are able to maybe extend it with filters, limits or offset in the future.

I'll purpose a slight change in the controller structure that might make this a little easier. I'll change it and put in a PR in a couple minutes. Let me know what you think when you have time.

@KenavR KenavR mentioned this issue Nov 8, 2017
@KenavR
Copy link
Member Author

KenavR commented Nov 8, 2017

I wrote my feedback in the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants