Add subscription transfer and contact deletion to CLI #443
Conversation
Nixolay
changed the title
cmd/cli/main.go
Outdated
| @@ -71,6 +77,14 @@ func main() { | |||
| logger.Errorf("Failed to enable images in all notifications") | |||
| } | |||
| } | |||
|
|
|||
| if err := transferUserSubscriptionsAndContacts(dataBase); err != nil { | |||
There was a problem hiding this comment.
Following style of another actions in this document it looks more clear to have something like
if *fromUser != "" && *toUser != "" {
if err := transferUserSubscriptionsAndContacts(dataBase, *fromUser, *toUser); err != nil {
logger.Error(err)
}
}There was a problem hiding this comment.
Another issue here is that tranfer of subscriptions require from user both toUser and fromUser. I think it will be better if we will check that only one of this valuea is specified and print warning message.
cmd/cli/main.go
Outdated
| logger.Error(err) | ||
| } | ||
|
|
||
| if err := deleteUser(dataBase); err != nil { |
There was a problem hiding this comment.
Same thing as in previous comment: can be rewritten in this way:
if *userDel != "" {
if err := deleteUser(dataBase, userDel); err != nil {
logger.Error(err)
}
}
cmd/cli/user.go
Outdated
| ) | ||
|
|
||
| func transferUserSubscriptionsAndContacts(database moira.Database) error { | ||
| if *fromUser == "" && *toUser == "" { |
There was a problem hiding this comment.
first my though after looking on this was: "What this variables mean and where are they declared?"
I think that passing this varibales in agruments will increase readability of this function.
cmd/cli/user.go
Outdated
| return database.SaveSubscriptions(subscriptions) | ||
| } | ||
|
|
||
| func deleteUser(database moira.Database) error { |
There was a problem hiding this comment.
Same comment as earlier. I will be more readable if we will receive user id here via function arguments.
cmd/cli/user.go
Outdated
| } | ||
| } | ||
|
|
||
| subscriptionIDs, err := database.GetUserSubscriptionIDs(*userDel) |
There was a problem hiding this comment.
Ok we decided that passing arguments as fromUser and toUser will be more clear instead of using global variables. But what should we do with userDel? I think that this should be another one argument for function.
Nixolay
force-pushed
the
feature/424-add-subscription-transfer-and-contact-deletion-cli
branch
from
0a9261e
to
7f10e8f
Compare
cmd/cli/main.go
Outdated
| @@ -71,6 +77,17 @@ func main() { | |||
| logger.Errorf("Failed to enable images in all notifications") | |||
| } | |||
| } | |||
| if *fromUser == "" && *toUser == "" { | |||
There was a problem hiding this comment.
Are you sure that here should be a == instead of !=? As I can see this expression is true only if fromUser and toUser are empty strings.
cmd/cli/main.go
Outdated
| } | ||
| } | ||
|
|
||
| if *userDel == "" { |
There was a problem hiding this comment.
Also probably should be !=
Nixolay
force-pushed
the
feature/424-add-subscription-transfer-and-contact-deletion-cli
branch
2 times, most recently
from
1590ed2
to
1ed0fe3
Compare
* Added flags for this feature: user-del, from-user, to-user * Create new functions transferUserSubscriptionsAndContacts and deleteUser Close #424
Nixolay
force-pushed
the
feature/424-add-subscription-transfer-and-contact-deletion-cli
branch
from
1ed0fe3
to
afe4895
Compare
beevee
requested review from
litleleprikon
and removed request for
borovskyav
beevee
deleted the
feature/424-add-subscription-transfer-and-contact-deletion-cli
branch
Close #424