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

Add #travel_to and a few more features #11

Merged
merged 7 commits into from
Dec 19, 2024
Merged

Add #travel_to and a few more features #11

merged 7 commits into from
Dec 19, 2024

Conversation

andrepiske
Copy link
Collaborator

@andrepiske andrepiske commented Dec 16, 2024

Jira task

This PR adds a few things, please see the changes in the CHANGELOG.md file (link which will eventually become broken).

@andrepiske andrepiske self-assigned this Dec 16, 2024
@andrepiske andrepiske changed the title Add versions at Add versions_at Dec 17, 2024
@@ -43,13 +43,13 @@
::IronTrailSpecMigrator.new.migrate

Time.now.tap do |date|
partition_name = "irontrail_chgn_#{date.strftime('%Y%m')}"
partition_name = "irontrail_chgn_infinite"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the goal isn't to test pg_party itself or partitioning. So since I'm now also using dates far in the past in some tests, I'll just create one big partition to accomodate them all.

@@ -19,6 +19,13 @@ def association_scope
scope
end

def reader
res = super
res.extend(CollectionProxyMixin)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wonder what YJIT thinks of this. Anyway, not a concern :)

@andrepiske andrepiske changed the title Add versions_at Add #travel_to and a few more features Dec 17, 2024
@andrepiske andrepiske marked this pull request as ready for review December 17, 2024 13:09
if val.length == 1
val[0]
else
val.to_h { |k| [k.to_s, k] }
Copy link
Member

Choose a reason for hiding this comment

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

Double-checking if this logic is correct, seems wrong to me considering the rest of the method. Also, I am unsure if you are and how hard it is to cover this path.

Suggested change
val.to_h { |k| [k.to_s, k] }
val.to_h { |k| [k.to_s, k] }

Copy link
Collaborator Author

@andrepiske andrepiske Dec 18, 2024

Choose a reason for hiding this comment

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

This is correct. I have to yet add simplecov to this project, won't do in this PR tho. This code is covered by the spec in spec/services/morpheus_spec.rb :)

This basically deals with tables that are mapped by multiple models, that is, activerecord Single Table Inheritance. So the mapping here becomes a hash mapping the model name to the model itself. The index variable will look something like:

index = {
  'some_sti_table' => {
    'ModelA' => ModelA,
    'ModelB' => ModelB
  },
  'some_other_table' => SomeOtherTable
}

Copy link
Member

@peterbrendel peterbrendel left a comment

Choose a reason for hiding this comment

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

@andrepiske andrepiske merged commit 3b3fff4 into main Dec 19, 2024
7 checks passed
@andrepiske andrepiske deleted the add-versions-at branch December 19, 2024 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants