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

Staging #69

Merged
merged 39 commits into from
Dec 11, 2023
Merged

Staging #69

merged 39 commits into from
Dec 11, 2023

Conversation

1nv8rzim
Copy link
Member

@1nv8rzim 1nv8rzim commented Dec 6, 2023

No description provided.

@1nv8rzim 1nv8rzim requested a review from a team as a code owner December 6, 2023 01:46
@1nv8rzim 1nv8rzim linked an issue Dec 6, 2023 that may be closed by this pull request
Copy link
Contributor

@jabbate19 jabbate19 left a comment

Choose a reason for hiding this comment

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

Suggestion to prevent issue: Prevent change while its your birthday or limit number of changes allowed

Comment on lines +75 to +88
entRemoveBirthdays, err := data.Birthday.GetBirthdays(yesterday.Day(), int(yesterday.Month()), internalSpan.Context())
if err != nil {
logging.Error(s, "failed to get yesterday's birthdays", nil, span, logrus.Fields{"error": err})
return
}

// Remove yesterday's birthdays
for _, entRemoveBirthday := range entRemoveBirthdays {
err = removeBirthday(s, entRemoveBirthday.Edges.User.ID, internalSpan.Context())
if err != nil {
logging.Error(s, "failed to remove birthday for "+helpers.AtUser(entRemoveBirthday.Edges.User.ID), nil, span, logrus.Fields{"error": err})
return
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Edge/Exploit Case: I move my birthday on my birthday. I wouldn't come up here

Copy link
Member Author

Choose a reason for hiding this comment

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

This is the scheduled task that adds and removes birthdays at midnight.

This edge case is covered in the snippets commented below

Comment on lines +246 to +262
now := time.Now().In(tz)
if month == int(now.Month()) && day == int(now.Day()) {
// respond to user that birthday cannot be today

err := s.InteractionRespond(i.Interaction, &discordgo.InteractionResponse{
Type: discordgo.InteractionResponseChannelMessageWithSource,
Data: &discordgo.InteractionResponseData{
Content: "You cannot set your birthday to be today",
Flags: discordgo.MessageFlagsEphemeral,
},
})
if err != nil {
logging.Error(s, "encounted error when responding to user", i.Member.User, span, logrus.Fields{"err": err.Error()})
}

return
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Birthday Edit or Add on Birthday Edge Case

Comment on lines +152 to +168
now := time.Now().In(tx)
if int(entBirthday.Month) == int(now.Month()) && int(entBirthday.Day) == int(now.Day()) {
// respond to user that birthday cannot be today

err := s.InteractionRespond(i.Interaction, &discordgo.InteractionResponse{
Type: discordgo.InteractionResponseChannelMessageWithSource,
Data: &discordgo.InteractionResponseData{
Content: "You cannot remove your birthday today",
Flags: discordgo.MessageFlagsEphemeral,
},
})
if err != nil {
logging.Error(s, "encounted error when responding to user", i.Member.User, span, logrus.Fields{"err": err.Error()})
}

return
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Birthday Delete on Birthday Edge Case

@1nv8rzim 1nv8rzim requested a review from jabbate19 December 7, 2023 03:45
Copy link
Contributor

@jabbate19 jabbate19 left a comment

Choose a reason for hiding this comment

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

:shipit:

@1nv8rzim 1nv8rzim merged commit 0d2265d into main Dec 11, 2023
3 checks passed
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.

Birthday Role
5 participants