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

feat: update goal REST no command #1464

Merged
merged 14 commits into from
Jan 21, 2025
Merged

feat: update goal REST no command #1464

merged 14 commits into from
Jan 21, 2025

Conversation

hvn2k1
Copy link
Contributor

@hvn2k1 hvn2k1 commented Jan 20, 2025

part of #1417

  • Implement REST update goal api
  • Implement goal e2e test

@hvn2k1 hvn2k1 marked this pull request as ready for review January 20, 2025 04:07
@hvn2k1 hvn2k1 marked this pull request as draft January 20, 2025 06:29
@hvn2k1 hvn2k1 force-pushed the update-goal-no-cmd branch from 098121c to c6e41af Compare January 20, 2025 07:21
@hvn2k1 hvn2k1 marked this pull request as ready for review January 20, 2025 08:51
Comment on lines 65 to 67
if deleted != nil {
updated.Goal.Deleted = deleted.Value
}
Copy link
Member

Choose a reason for hiding this comment

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

If the goal is not in use, we can delete it from the table. There is no need to do a soft delete.

@hvn2k1 hvn2k1 force-pushed the update-goal-no-cmd branch from 395dfdd to d2453c7 Compare January 21, 2025 07:20
name *wrapperspb.StringValue,
description *wrapperspb.StringValue,
archived *wrapperspb.BoolValue,
deleted *wrapperspb.BoolValue,
Copy link
Member

Choose a reason for hiding this comment

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

You can remove this since you are not using in the update API anymore.

Suggested change
deleted *wrapperspb.BoolValue,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have remove the redundant code!

@hvn2k1 hvn2k1 force-pushed the update-goal-no-cmd branch from d2453c7 to 84e7fde Compare January 21, 2025 07:44
Copy link
Member

@cre8ivejp cre8ivejp left a comment

Choose a reason for hiding this comment

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

Nice!
Thank you!

@hvn2k1 hvn2k1 merged commit aeabf7e into main Jan 21, 2025
17 checks passed
@hvn2k1 hvn2k1 deleted the update-goal-no-cmd branch January 21, 2025 08:20
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.

2 participants