Skip to content

Issue169 #183

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

Merged
merged 7 commits into from
Aug 30, 2016
Merged

Issue169 #183

merged 7 commits into from
Aug 30, 2016

Conversation

louisdelanoeye
Copy link
Contributor

added delete and modify button to examples

<strong><%= exampleUser.username %></strong>
at
<div class="example-wrapper">
<% if(reviewTarget!==null){ %>
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe rename this exampleTarget, since it is not related to review anymore

@ludov04
Copy link
Contributor

ludov04 commented Aug 29, 2016

Can you rebase it when you've done all the changes, so I can merge it ?

@@ -126,6 +126,8 @@ module.exports.routes = {
// API
'get /api/topics/:topicId/examples': 'ExampleController.findByTopic',
'post /api/topics/:topicId/examples': 'ExampleController.postExampleToTopic',
'post /api/:exampleId/remove': 'ExampleController.deleteExample',
'post /api/:exampleId/update': 'ExampleController.updateExample',
Copy link
Contributor

@ludov04 ludov04 Aug 30, 2016

Choose a reason for hiding this comment

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

@louisdelanoeye
So the route looks like this: /api/1/remove ?
It's not clear that we deal with examples.

I would rename theses routes like this:
delete /api/examples/:exampleId': 'ExampleController.deleteExample
post /api/examples/:exampleId': 'ExampleController.updateExample

No need for the /remove and /update the http method tells us the action to do

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