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: New project events #1874

Merged
merged 3 commits into from
Jul 4, 2024
Merged

feat: New project events #1874

merged 3 commits into from
Jul 4, 2024

Conversation

ncomerci
Copy link
Contributor

@ncomerci ncomerci commented Jul 3, 2024

This closes #1776

@ncomerci ncomerci requested review from 1emu and andyesp July 3, 2024 16:52
}

export async function down(): Promise<void> {
return
Copy link
Collaborator

Choose a reason for hiding this comment

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

shouldn't you remove those event types?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not recommended to do that (check here)

@@ -209,6 +210,7 @@ export async function finishProposal() {
BadgesService.giveFinishProposalBadges(proposalsWithOutcome)
DiscourseService.commentFinishedProposals(proposalsWithOutcome)
DiscordService.notifyFinishedProposals(proposalsWithOutcome)
await EventsService.proposalFinished(proposalsWithOutcome)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we need to await this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

why not? it's an async method

@@ -306,6 +306,7 @@ export class ProposalService {
const project = await ProjectService.getUpdatedProject(proposal.project_id!)
updatedProposal.project_status = project.status
NotificationService.projectProposalEnacted(proposal)
await EventsService.projectEnacted(project)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here, no need for await

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's also an async method 🤔

@@ -354,6 +359,43 @@ export class EventsService {
}
}

static async proposalFinished(proposalsWithOutcome: ProposalWithOutcome[]) {
for (const proposal of proposalsWithOutcome) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm wondering if we should try catch inside the for... wdyt?

@andyesp andyesp merged commit 05a963c into master Jul 4, 2024
2 checks passed
@andyesp andyesp deleted the feat/new-project-events branch July 4, 2024 14:05
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.

Projects Sheet: Activity
3 participants