Fix Duplicate Sketch button #1683
Conversation
| .then(res => res.json()) | ||
| .then(data => resolve({ | ||
| files: data.files, | ||
| const currentUser = getState().user.username; |
There was a problem hiding this comment.
I don't think this is the right solution—what if a user is duplicating a sketch that doesn't belong to them?
There was a problem hiding this comment.
I see...yes I hadn't thought of that, sorry!
I went through the GET routes specified on the server side and the appropriate one appeared to be router.get('/:username/projects/:project_id', ...), so I thought of implementing it this way.
There was a problem hiding this comment.
Sketches/projects (you can use the terms interchangeably) are unique by id so you shouldn't need to include the username. I don't think this is the reason it's failing—I would look at how it's different when calling cloneProject from SketchList.jsx versus Nav.jsx.
There was a problem hiding this comment.
I agree the route should remain '/projects/:project_id', however, I couldn't find an implementation that would allow fetching of projects only by their ID. So I took the liberty of adding another controller and GET route for the same.
Could you please take a look?
|
Thanks for working on this! In reviewing your code, I realized it didn't make any sense to make the extra API call when frontend already has the sketch, so I made a new PR: #1686. Want to take a look at that one? |
Ah of course. The new PR looks good! |
2 participants
Fixes #1677
I have verified that this pull request:
npm run lint)developbranch. (If I was asked to make more changes, I have made sure to rebase ontodevelopthen too)Fixes #123@catarak kindly let me know if the changes are alright.