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

Implement update endpoint for Files, Nodes, Notes, Reminders and Timelines module #43

Open
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

HunorTotBagi
Copy link
Collaborator

@HunorTotBagi HunorTotBagi commented Jan 7, 2025

In this PR I did the following things:

  • Created update endpoint, command and handler for Files, Nodes, Notes, Reminders and Timelines module
  • Added the update requests to postman collection
  • Added validation for Id field for every update command
  • Tested the PUT endpoint for:
    • Files module
    • Nodes module
    • Notes module
    • Reminders module
    • Timelines module

@HunorTotBagi HunorTotBagi added enhancement New feature or request backend Work performed on the backend solution labels Jan 7, 2025
@HunorTotBagi HunorTotBagi self-assigned this Jan 7, 2025
@HunorTotBagi HunorTotBagi marked this pull request as ready for review January 8, 2025 07:30
Copy link
Owner

@NikolaVetnic NikolaVetnic left a comment

Choose a reason for hiding this comment

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

It should not be mandatory for the user to send all the entity information in order to update the entity. For example, sending this should also work and it should update only the title:

{
    "Note": {
        "Id": "74dad71c-4ddc-4d4d-a894-3307ddc3fe10",
        "Title": "This title was AGAIN updated from Postman."
    }
}

At the moment I am getting the validation error which makes no sense. I can think of a possible solution, but will give you time to come up with your own one first.

@@ -1,6 +1,6 @@
namespace Notes.Application.Entities.Notes.Commands.UpdateNote;

public record UpdateNoteCommand(NoteDto Note) : ICommand<UpdateNoteResult>;
public record UpdateNoteCommand(UpdateNoteDto Note) : ICommand<UpdateNoteResult>;
Copy link
Owner

Choose a reason for hiding this comment

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

This is what I had in mind... Sort of 😄 As it is here, the command serves simply as a wrapper around the UpdateNoteDto type, and that DTO is again used here and only here.

Do you have an idea on what a better solution would be?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Work performed on the backend solution enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants