Skip to content

Conversation

@kjriga
Copy link
Contributor

@kjriga kjriga commented Oct 3, 2025

Porting over the MediaMail calculator from TJ. I added a seed file to ensure the required shipping category can be made if desired, to use this calculator.

@kjriga kjriga force-pushed the kendra/tj-217-write-solidususpscalculatormediamail-calculator branch 5 times, most recently from a8d5133 to 54b683f Compare October 6, 2025 17:19
@kjriga kjriga marked this pull request as ready for review October 6, 2025 17:23
@kjriga kjriga force-pushed the kendra/tj-217-write-solidususpscalculatormediamail-calculator branch from 54b683f to e2e09f5 Compare October 6, 2025 17:24
@kjriga kjriga changed the title wip Create MediaMail calculator Oct 6, 2025
Copy link
Member

@benjaminwil benjaminwil left a comment

Choose a reason for hiding this comment

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

Looks good. I have a couple suggestions:

  • Less redundant constant name (see additional comment)
  • Fix the missing commit message (wip)

module SolidusUsps
module Calculator
class MediaMail < SolidusUsps::Calculator::Base
MEDIA_MAIL_CATEGORY_NAME = "Media Mail".freeze
Copy link
Member

Choose a reason for hiding this comment

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

Nit: This constant is defined the MediaMail class so MEDIA_MAIL_ is a redundant prefix in the end. Suggestion:

- SolidusUsps::Calculator::MediaMail::MEDIA_MAIL_CATEGORY_NAME
+ SolidusUsps::Calculator::MediaMail::CATEGORY_NAME

@kjriga kjriga force-pushed the kendra/tj-217-write-solidususpscalculatormediamail-calculator branch 3 times, most recently from c693cb0 to 6b64c90 Compare October 6, 2025 19:43
@kjriga
Copy link
Contributor Author

kjriga commented Oct 6, 2025

Fixed Benjamin's comments. I also updated this calculator to preemptively add the new mail_class method so it doesn't get missed, since that change is in PR at the same time as this.

@kjriga kjriga force-pushed the kendra/tj-217-write-solidususpscalculatormediamail-calculator branch from 6b64c90 to 3a1b421 Compare October 6, 2025 19:45
From TJ, we need this. It relies on shipping category for calculating
availability, which I added a seed and README info to.
@kjriga kjriga force-pushed the kendra/tj-217-write-solidususpscalculatormediamail-calculator branch from 3a1b421 to 5b43bea Compare October 6, 2025 19:46
@kjriga kjriga merged commit 3d9b3b5 into main Oct 6, 2025
2 checks passed
@adammathys adammathys deleted the kendra/tj-217-write-solidususpscalculatormediamail-calculator branch October 7, 2025 17:10
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