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

chore!: refactoring quiz for it to be time based #3732

Merged
merged 9 commits into from
Dec 21, 2023

Conversation

nicolasburtey
Copy link
Member

@nicolasburtey nicolasburtey commented Dec 19, 2023

  • removing earn and rewards as part of the naming, converging to quiz (using Quiz because this is what the API layer is using)
  • moving the data out of the Account collection into its own collection
    -- will make it easier to parse later once and have rules based on time
    -- will make it easier to extract to an independent pod if we choose to in the future

@github-actions github-actions bot added the core label Dec 19, 2023
@nicolasburtey nicolasburtey force-pushed the feat--time-based-earn branch 5 times, most recently from 86d9828 to 8b22994 Compare December 20, 2023 22:54
@nicolasburtey nicolasburtey changed the title Feat: time based earn chore: refactoring quiz for it to be time based Dec 20, 2023
).authorize(undefined)
expect(validator).toBeInstanceOf(ExpectedPhoneMetadataMissingError)
})

it("returns error with voip type", () => {
const validator = PhoneMetadataAuthorizer(
getPhoneMetadataRewardsSettings(),
getPhoneMetadataQuizzesSettings(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

quizzes or quiz?

Copy link
Member Author

Choose a reason for hiding this comment

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

it depend of the context on whether it's singular or plural

core/api/src/services/mongoose/quiz.ts Outdated Show resolved Hide resolved
}
}

return {
Copy link
Collaborator

Choose a reason for hiding this comment

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

please add tracing wrapper (at least for add)

Copy link
Member Author

Choose a reason for hiding this comment

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

isn't the tracing already provided by mongodb tracing module? we are not doing this for the other module in this mongoose folder

core/api/src/services/quiz/index.ts Outdated Show resolved Hide resolved
core/api/src/services/mongoose/schema.ts Show resolved Hide resolved
@@ -7,7 +7,7 @@ export class PhoneCarrierTypeNotAllowedError extends UnauthorizedPhoneError {}
export class PhoneCountryNotAllowedError extends UnauthorizedPhoneError {}

export class InvalidPhoneForOnboardingError extends UnauthorizedPhoneError {}
export class InvalidPhoneForRewardError extends UnauthorizedPhoneError {}
export class InvalidPhoneForQuizError extends UnauthorizedPhoneError {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

please be consistent Quiz or Quizzes (I prefer quiz but it is up to you)

key: RateLimitPrefix.addEarnAttemptPerIp,
limits: getAddEarnPerIpLimits(),
error: UserAddEarnAttemptIpRateLimiterExceededError,
addQuizAttemptPerIp: {
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, quiz or quizzes?

core/api/src/config/schema.types.d.ts Outdated Show resolved Hide resolved
@@ -13,6 +13,7 @@ export * from "./schema"
import { ConfigError } from "./error"

import { toDays } from "@/domain/primitives"
import { QuizzesValue } from "@/app/quiz/config"
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 thits be part of config and not from app layer? it is creating a circular dependency

Copy link
Member Author

Choose a reason for hiding this comment

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

not sure, but I've also been thinking about that. I think this is the config of the quizzes, so we may argue this could be fine? I also try to make the quiz module for self contain.

core/api/src/app/quiz/config.ts Outdated Show resolved Hide resolved
@nicolasburtey nicolasburtey force-pushed the feat--time-based-earn branch 3 times, most recently from 639abc3 to 667b2ea Compare December 21, 2023 01:03
@nicolasburtey nicolasburtey changed the title chore: refactoring quiz for it to be time based chore!: refactoring quiz for it to be time based Dec 21, 2023
@nicolasburtey nicolasburtey marked this pull request as ready for review December 21, 2023 01:07
core/api/src/services/mongoose/quiz.ts Outdated Show resolved Hide resolved
core/api/src/services/mongoose/quiz.ts Outdated Show resolved Hide resolved
@nicolasburtey nicolasburtey merged commit acfcd79 into main Dec 21, 2023
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants