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

Initialize and Protect Problem Count #39

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Conversation

idodin
Copy link
Member

@idodin idodin commented Jun 12, 2020

Initializes Problem Count field upon creation.
Protects Problem Count from modification on update endpoint.
resolve #33
resolve #14

Initializes Problem Count field upon creation.
Protects Problem Count from modification on update endpoint.
resolve #33
resolve #15
@ValerianClerc
Copy link
Member

Could we hold off on merging this until I merge the last of the problems tests? I did a lot of refactoring of the controllers, regarding updating problemCount, so there will be changes to make

@idodin
Copy link
Member Author

idodin commented Jun 12, 2020

Sure, will wait :p

Comment on lines +321 to +325
const updatedUser = JSON.parse(JSON.stringify(testProblemSet1));
updatedUser._id = req.params.problemSetId;
updatedUser.title = "Test Problem Set Title 1 Updated";
updatedUser.problemCount = 9;
stubs.problemSetDB.update.resolves(updatedUser);
Copy link
Member

Choose a reason for hiding this comment

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

Confusing variable name updatedUser. Change to updatedProblemSet.
Also, why do you need to stringify then parse? Isn't testProblemSet1 already a javascript object (IProblemSet)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Kept both in line with test that precedes - will change after I update with Val's changes.

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.

ProblemCount shouldn't be manually updateable Initialize problemCount upon creating a new problemSet
3 participants