SE701-T5 / Backend

Heroku deployed Node.js backend server developed by Team-5 for the UniForum project associated with the University of Auckland SOFTENG 701 course
https://uni-forum.herokuapp.com/
MIT License
2 stars 18 forks source link

Implement Forum Post Update Feature using HTTP PATCH #53

Closed R055A closed 2 years ago

R055A commented 2 years ago

Description

Implement update feature that responds to HTTP PATCH requests for updates of any valid editable forum post document field(s) with the updated forum post data.

Refactored existing forum controller functions to use added validation functionality.

Related Issue

Solves #27

Type of change

How Has This Been Tested?

Multiple automated test cases for

Checklist:

A similar pull request already exists for this, but it is made by the same GitHub user. The reason it is duplicated is to fix an issue with commits from prior PRs that aren't yet merged being added to new PRs which is confusing reviewers. To fix the issue, the repository fork the initial PR was made from was deleted. When that fork reposotiry was deleted the initial PR automatically closed and cannot be merged because of this. In the past when I delete a fork repository the PRs from that repository have not been closed, but in this case the PR was closed. A new fork was made to the same GitHub account. The same changes have been pushed to the new fork repository and a duplicate PR has been required for merging those changes. The now closed initial PR is #50

R055A commented 2 years ago

Big work! But a lot going on in this PR. Wouldn't it be better to split up the work amongst multiple PRs? Also, looks like there was only one big commit instead of multiple small commits. This is just what I'm thinking so let me know if there were any particular reasons for implementing it in this way.

I implemented the feature described in the issue. I added test cases for this. I did some reafctoring to related existing features that are already merged and no longer being worked on my anyone. I refactored to reduce code reuse by adding validation functions and added calls to these functions in place of where code was being repeated.

Regarding the changes to user.controller, user.model and user.test, I am unaware why they have been included. I will remove them. Thank you for bringing this to my attention.

R055A commented 2 years ago

Big work! But a lot going on in this PR. Wouldn't it be better to split up the work amongst multiple PRs? Also, looks like there was only one big commit instead of multiple small commits. This is just what I'm thinking so let me know if there were any particular reasons for implementing it in this way.

I implemented the feature described in the issue. I added test cases for this. I did some reafctoring to related existing features that are already merged and no longer being worked on my anyone. I refactored to reduce code reuse by adding validation functions and added calls to these functions in place of where code was being repeated.

Regarding the changes to user.controller, user.model and user.test, I am unaware why they have been included. I will remove them. Thank you for bringing this to my attention.

The unwanted changes are now removed. The only changes that remain in any file relating to the user features are the addition of empty lines at the bottom of a file, the removal of an unwanted empty line at the top of a file and the removal of unwanted empty spaces at the beginning of functions declarations.