[MM-870]: Fixed issue of deletion of non-existent subscription #886
Conversation
Kshitij-Katiyar
changed the title
Kshitij-Katiyar
changed the base branch from
master
to
project-board-issue-workflow
Kshitij-Katiyar
changed the base branch from
project-board-issue-workflow
to
master
webapp/src/components/modals/create_issue/__snapshots__/create_issue.test.jsx.snap
Outdated
Show resolved
Hide resolved
Yash-Chakerverti
left a comment
Yash-Chakerverti
left a comment
There was a problem hiding this comment.
This PR is tested & all the code changes are working fine.
LGTM
abbas-dependable-naqvi
requested review from
hanzei
and removed request for
hanzei
|
@hanzei we need to merge this to fix a bug. There is a ticket to add the tests, which has been our process to deal with the fact that MM released these plugins with next to no test coverage at all. The coverage has much improved as the tickets do get worked on. |
hanzei
left a comment
hanzei
left a comment
There was a problem hiding this comment.
One non blocking suggestion
| const ( | ||
| SubscriptionNotFound = iota | ||
| SubscriptionAlreadyExists | ||
| InternalServerError | ||
| ) | ||
|
|
||
| func NewSubscriptionError(code int, err error) *SubscriptionError { | ||
| return &SubscriptionError{Code: code, Error: err} | ||
| } |
There was a problem hiding this comment.
2/5 I prefer using customer error variables and then checking them using errors.Is. That is what the std lib recommends and is also used on the MM server.
There was a problem hiding this comment.
The Go devs think typed errors are better than sentinel errors. The reason they are not changed in the std lib are for compatibility. Also sentinel errors are much slower.
One thing I aways disliked about sentinel errors is that code could replace the error var with something else. MM server is a mix of sentinel and typed errors.
Summary
Fixed the issue of deletion of a non-existent subscription
Screenshot
Old
New
Issue link
Fixes #870