-
Notifications
You must be signed in to change notification settings - Fork 16
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
Update Categories #276
base: main
Are you sure you want to change the base?
Update Categories #276
Conversation
0de2d38
to
142c1ab
Compare
|
||
UpdateCategoryRequest updateRequest = new() | ||
{ | ||
Name = "new update", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test updating the other fields too.
{ | ||
Email = email, | ||
Password = "Passw0rd", | ||
Username = $"CreateCatTest{role}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change this to UpdateCatTest{role}
.
|
||
if (slug is not null) | ||
{ | ||
problemDetails!.Errors["Slug"].ToArray().Should().Equal([SlugRule.SLUG_FORMAT]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the ToArray() call needed here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guess it wasn't. Thought it was.
[TestCase(1, "b.b")] | ||
[TestCase(2, "b")] | ||
[TestCase(3, null)] | ||
public static async Task UpdateCategory_BadData(int index, string? slug) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the index
param doing here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To act as a differentiator for the cats I create. Can't add slug
to the cats' slugs
[SwaggerResponse( | ||
409, | ||
"The specified slug is already in use by another category. Returns the conflicting category.", | ||
typeof(CategoryViewModel) | ||
)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we use a Conflict Details here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yeah of course. Sorry it slipped my mind. (Peer reviewing really does work, after all)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a test where you try to update a category's Type and it gives an error.
6808359
to
409025f
Compare
409025f
to
e9fee4f
Compare
- Change UpdateCategory 409 to return a ConflictDetails - Change UpdateCategory tests to directly call DB instead for scaffolding
e9fee4f
to
fcebb97
Compare
Closes: #263
Depends: #275