Skip to content

fix: use original types for composite primary keys#246

Merged
olavloite merged 5 commits intogoogleapis:mainfrom
nownabe:non-number-interleave-key
May 15, 2023
Merged

fix: use original types for composite primary keys#246
olavloite merged 5 commits intogoogleapis:mainfrom
nownabe:non-number-interleave-key

Conversation

@nownabe
Copy link
Contributor

@nownabe nownabe commented May 8, 2023

Close #245

@nownabe nownabe requested review from a team and olavloite as code owners May 8, 2023 05:56
@product-auto-label product-auto-label bot added the api: spanner Issues related to the googleapis/ruby-spanner-activerecord API. label May 8, 2023
@nownabe nownabe force-pushed the non-number-interleave-key branch from 8860f59 to 33dc0ab Compare May 8, 2023 07:04
@nownabe nownabe force-pushed the non-number-interleave-key branch from 33dc0ab to 880716c Compare May 9, 2023 01:50
@nownabe
Copy link
Contributor Author

nownabe commented May 9, 2023

I added a new test helper to allow us to create more flexible tests and to bundle related test code.
I intend to use this helper for other tests as well.

Copy link
Collaborator

@olavloite olavloite left a comment

Choose a reason for hiding this comment

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

Thanks @nownabe! This generally looks good to me. I had a couple of minor nits and requests for tests, but feel free to add that in a separate PR if that is easier.

end
end

def test_create_album
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would you mind adding a couple of tests for other basic operations as well:

  • UpdateAlbum
  • GetAlbum

And also verify that it is possible to get the albums of a singer (that is; iterate over the collection of albums of a singer)

@@ -0,0 +1,41 @@
require "securerandom"
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: add a copyright header

You can copy one from any of the other files. Please make sure to update the year to 2023

@nownabe
Copy link
Contributor Author

nownabe commented May 15, 2023

@olavloite Thank you for your review! I updated them.

@olavloite olavloite merged commit a11b3b1 into googleapis:main May 15, 2023
@nownabe nownabe deleted the non-number-interleave-key branch May 15, 2023 05:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: spanner Issues related to the googleapis/ruby-spanner-activerecord API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Interleaves don't work using non-number parent keys on Rails 7

2 participants