Skip to content

Conversation

pablo-code-org
Copy link
Contributor

@pablo-code-org pablo-code-org commented Aug 11, 2022

  • This splits the library for pegasus and dashboard so they consume the data from different data sources.

Links

Testing story

Deployment strategy

Follow-up work

Privacy

Security

Caching

PR Checklist:

  • Tests provide adequate coverage
  • Privacy and Security impacts have been assessed
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

@pablo-code-org pablo-code-org requested review from davidsbailey and a team August 15, 2022 16:29
@pablo-code-org pablo-code-org marked this pull request as ready for review August 15, 2022 16:30

if default_random_donor && !donor_name
donor = CdoDonor.get_random_donor_by_weight
donor = PegasusCdoDonor.get_random_donor_by_weight
Copy link
Member

Choose a reason for hiding this comment

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

hey @pablo-code-org , the challenge here is that certificate_image.rb is used by both pegasus and dashboard. Due to some quirks in the way we run pegasus locally and in drone, I have some concerns about this particular change sneaking through drone but then breaking in the DTT. If you want to proceed with this, I would suggest spinning up an adhoc and making sure the hour_of_code_finish.feature UI test is passing there.

alternately, what we had talked about at the end of last week was putting this change on hold until the congrats work is done, which I am planning to complete in the next few weeks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I'll leave the PR ready and wait for the image certificate changes

@cat5inthecradle
Copy link
Contributor

@pablo-code-org @davidsbailey Did the blocking work get completed and is this ready to be picked back up?

@davidsbailey
Copy link
Member

@cat5inthecradle no, the blocking work is still in progress, currently actively being worked on.

@davidsbailey
Copy link
Member

davidsbailey commented Oct 21, 2022

I got eager and merged latest staging (including the blocking congrats changes) into this PR 😝 after resolving merge conflicts and changing one PegasusCdoDonors to DashboardCdoDonors I think it's good to go! @sureshc would you mind also reviewing since now pablo and I have both contributed commits to this PR?

@davidsbailey davidsbailey requested a review from sureshc October 21, 2022 18:18
Conflicts:
	dashboard/app/helpers/application_helper.rb
@davidsbailey davidsbailey merged commit 7a1342c into staging Oct 24, 2022
@davidsbailey davidsbailey deleted the redirect_donors_2 branch October 24, 2022 21:38
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.

3 participants