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

Era week number test #84

Merged
merged 43 commits into from
Jan 6, 2021
Merged

Era week number test #84

merged 43 commits into from
Jan 6, 2021

Conversation

LHJE
Copy link
Collaborator

@LHJE LHJE commented Jan 5, 2021

Code Highlights:

  • Updates Users Factory to make the premade user older, making testing easier.
  • Updates Eras and Events Factories to have week_number
  • Updates the create_era.rb to accept the week_number argument
  • Updates the create_era.rb to calculate week_number
  • Updates the update_era.rb to accept the week_number argument
  • Updates the update_era.rb to calculate week_number
  • Updates the create_era_spec.rb to take week_number into consideration
  • Updates the update_era_spec.rb to take week_number into consideration
  • Updates the get_single_era_spec.rb to test weekNumber being exposed

Where should the reviewer start?

  • create_era_spec.rb and update_era_spec.rb

Have Tests Been Added?

  • No.
  • Yes, but all tests are not passing.
  • Yes, and all are passing.

Any background context you want to provide?

Message/Questions for reviewer:

  • The calculation for which week the era starts is not to-the-date perfect, because it's only taking the difference between the start date of the era and the user's bday, but it's a start. It'll have to be refined in the future, but it is working for now.

Issues:

Screenshots (if appropriate):

Tracking Consistency:

  • added appropriate labels
  • My code follows the code style of this project and has removed all unnecessary annotations
  • I have added comments on my pull request, particularly in hard-to-understand areas
  • looked at PR preview to check spelling, syntax, formatting, and completion
  • Rubocop Violations: Just the get_users one

@LHJE LHJE added this to In progress in Eras via automation Jan 5, 2021
Copy link
Collaborator

@AngelaGuardia AngelaGuardia left a comment

Choose a reason for hiding this comment

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

This looks good @LHJE! Left some comments for some changes.

app/graphql/mutations/eras/create_era.rb Outdated Show resolved Hide resolved
app/graphql/mutations/eras/create_era.rb Outdated Show resolved Hide resolved
app/graphql/mutations/eras/update_era.rb Outdated Show resolved Hide resolved
app/graphql/mutations/eras/create_era.rb Outdated Show resolved Hide resolved
app/graphql/types/era_type.rb Outdated Show resolved Hide resolved
spec/factories/eras.rb Outdated Show resolved Hide resolved
spec/factories/events.rb Outdated Show resolved Hide resolved
spec/factories/users.rb Show resolved Hide resolved
Eras automation moved this from In progress to Review in progress Jan 6, 2021
Copy link
Collaborator Author

@LHJE LHJE left a comment

Choose a reason for hiding this comment

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

Thanks for the comments! Looks great!

@leahriffell leahriffell merged commit 23011a1 into main Jan 6, 2021
Eras automation moved this from Review in progress to Done Jan 6, 2021
@leahriffell leahriffell deleted the era_week_number_test branch January 6, 2021 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Eras
Done
4 participants