-
Notifications
You must be signed in to change notification settings - Fork 4
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
Fixes #259 - Implements is_first_version
and is_last_version
attributes
#287
Conversation
# HACK: reload item - this is a poor version of Persistence::reload | ||
# https://github.com/rails/rails/blob/1f98eb60e59f4f70ef66ac2454ad029f46e3b27c/activerecord/lib/active_record/persistence.rb#L432 | ||
# The only difference is the `unscoped` method is removed. | ||
def reload(options = nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See:
- https://github.com/QutBioacoustics/baw-server/pull/287/files/4db980b4531b65d47e730247b0e0c6b7067ec239#diff-9cacd63540753d6929377d6cb58210cbR157
- https://github.com/QutBioacoustics/baw-server/pull/287/files/4db980b4531b65d47e730247b0e0c6b7067ec239#diff-42b71321a35f19e8409b3b39966e6f24R62
- https://github.com/QutBioacoustics/baw-server/pull/287/files/4db980b4531b65d47e730247b0e0c6b7067ec239#diff-42b71321a35f19e8409b3b39966e6f24R69
Because of scoped query needs to be count(:all)
So I'm reviewing the changes in 0ab8f91. Converting the Please consider the changes in 8bc7cb2 which use the attribute values if they are present and query for them in the rare case they are not present. |
That looks reasonable - have you tested your most recent change much? |
Unit and integration tests pass. Basic tests with http endpoint tester behave as I expect for |
cool, I'll review and merge tonight |
Adds virtual fields
is_first_version
andis_last_version
to theScripts API. Does NOT implement the
/groups
API endpoint - unnecessaryat this time.
Note one unit test currently fails because #286 is not implemented.