Skip to content

changelog: Internal, Analytics, Fix weekly protocols report#11406

Merged
Sgtpluck merged 2 commits intomainfrom
dmm/fix-protocols-report
Oct 28, 2024
Merged

changelog: Internal, Analytics, Fix weekly protocols report#11406
Sgtpluck merged 2 commits intomainfrom
dmm/fix-protocols-report

Conversation

@Sgtpluck
Copy link
Contributor

🛠 Summary of changes

Last week's update of the ProtocolsReport led to an Argument Error when the cron job attempted to run.

I'm not totally sure how the GoodJob cron jobs work, but I believe it's because I added an initializer method for the Mailer Preview feature, and that the report_date argument was being passed into that rather than the perform method. There may be a different way to solve this, but I wasn't sure how to test it.

def perform(date = Time.zone.yesterday.end_of_day)
return unless IdentityConfig.store.s3_reports_enabled

self.report_date = report_date
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd remove the initializer just in case and use the attr_accessor for specs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the ReportMailerPreview needs the date added via initializer, since it bypasses the perform method :(

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could do

    report = Reports::ProtocolsReport.new.tap { |r| r.report_date = date }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated in 42711be

@mitchellhenke
Copy link
Contributor

The previous PR is here: #11369

@Sgtpluck Sgtpluck merged commit 01e1a1c into main Oct 28, 2024
@Sgtpluck Sgtpluck deleted the dmm/fix-protocols-report branch October 28, 2024 17:17
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