Skip to content

Update RR settings to support string-based date ranges#158

Merged
saikumar9 merged 13 commits intotrunkfrom
update_rr_settings
Jul 3, 2025
Merged

Update RR settings to support string-based date ranges#158
saikumar9 merged 13 commits intotrunkfrom
update_rr_settings

Conversation

@jayreddy519
Copy link
Contributor

@jayreddy519 jayreddy519 commented Jun 30, 2025

PR enhances the ResourceRegistry::Setting class to support string-based date ranges in item attribute.

ERROR:

NoMethodError: undefined method `cover?' for String

SOLUTION:

Centralized the fix in the correct place (ResourceRegistry::Setting)
Automatically handle "MM/DD/YYYY..MM/DD/YYYY" and "YYYY-MM-DD..YYYY-MM-DD" formats

This update overrides the item method in ResourceRegistry::Setting to normalize string-based date ranges.
If the item value is a String in the format "MM/DD/YYYY..MM/DD/YYYY" or "YYYY-MM-DD..YYYY-MM-DD", it is parsed and converted into a proper Range

This change maintains backward compatibility && requires no updates on Enroll.

TESTING:
Tested this change on ENROLL with branch - fix_trunk_failures_7_1_2025.
No rspec errors related to cover? Date string

@jayreddy519 jayreddy519 added the bugfix Something isn't working label Jun 30, 2025
@jayreddy519 jayreddy519 requested a review from Copilot July 2, 2025 14:33
@jayreddy519 jayreddy519 changed the title Update RR settings Update RR settings to support string-based date ranges Jul 2, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances ResourceRegistry::Setting to automatically convert supported string-based date ranges into Range<Date> objects and adds corresponding specs.

  • Override item accessor to normalize date-range strings
  • Implement convert_range_strings, looks_like_date?, and parse_date helpers
  • Add RSpec tests covering various date-range and non–date-range scenarios

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
spec/resource_registry/setting_spec.rb Added tests for parsing and preserving different item values
lib/resource_registry/setting.rb Overrode item and added string-to-date-range conversion logic
Comments suppressed due to low confidence (2)

lib/resource_registry/setting.rb:47

  • [nitpick] Returning nil for partially parsed date ranges may introduce unexpected nil values downstream. Consider returning the original string or raising a descriptive error to make failures explicit.
          return nil

spec/resource_registry/setting_spec.rb:87

  • [nitpick] The test description says it returns nil for non-date ranges, but the expectation asserts the original string. Update the description to reflect that it returns the original string.
    it "returns nil for non-date ranges" do

@jayreddy519 jayreddy519 force-pushed the update_rr_settings branch from b282ece to f9bec13 Compare July 2, 2025 15:13
Copy link
Member

@saikumar9 saikumar9 left a comment

Choose a reason for hiding this comment

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

lgtm!

@saikumar9 saikumar9 enabled auto-merge (squash) July 3, 2025 14:04
Copy link

@jacobkagon jacobkagon left a comment

Choose a reason for hiding this comment

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

LGTM!

@saikumar9 saikumar9 merged commit 920af75 into trunk Jul 3, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants