-
Notifications
You must be signed in to change notification settings - Fork 22
Precise timestamp filtering and sorting for created/modified fields #80
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
base: 2.x
Are you sure you want to change the base?
Conversation
xispa
left a comment
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.
Thanks @nnpvaan . I think that you can simplify a lot, please note the comment.
src/senaite/jsonapi/catalog.py
Outdated
| append = filtered.append | ||
| for brain in results: | ||
| # Get the actual object to access created/modified | ||
| obj = brain.getObject() |
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.
Instead of iterating over the brains and building a list of tuples like (brain, created, modified), I suggest to sort directly the brains using the sorted. Also note that you can rely on the get_creation_date and get_modification_date helpers from bika.lims.api, which extract the date from the brain first and fall back to the object only when the metadata is missing. The key advantage of this approach is that it avoids waking up the underlying objects unless absolutely necessary.
Something like this:
sort_on = query.get("sort_on")
reverse = query.get("sort_order") == "descending"
# DateIndex only supports minute-level precision, so we must manually sort the results
# to achieve second-level accuracy when ordering by creation or modification time.
if sort_on == "created":
brains = sorted(results, key=lambda b: api.get_creation_date(b), reverse=reverse)
if sort_on == "modified":
brains = sorted(results, key=lambda b: api.get_modification_date(b), reverse=reverse)
return brainsThere 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.
Also note that you can rely on the get_creation_date and get_modification_date helpers from bika.lims.api, which extract the date from the brain first and fall back to the object only when the metadata is missing
We can’t reliably use get_creation_date or get_modification_date helpers on catalog brains, because those attributes may be Missing.Value if not present in the catalog metadata (that's why it does not work so far)

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.
We can’t reliably use
get_creation_dateorget_modification_datehelpers on catalog brains, because those attributes may beMissing.Valueif not present in the catalog metadata (that's why it does not work so far)
The created and modified metadata retrieved from catalog brains should never return Missing.Value. All objects, both Dexterity (DX) and Archetypes (AT), provide created() and modified() functions, which always return a DateTime.
Therefore, if Missing.Value is encountered, it indicates that the metadata columns were added to the catalog (so Missing.Value was set by default), but objects were not reindexed afterwards. To resolve this, you can reindex the objects of the affected catalog directly from the Zope Management Interface (Site Setup > Management Interface). Navigate to the "Advanced" tab and click "Update Catalog".
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.
@xispa Thank you for your help. It works after I updated the catalog. I switched to using the get_creation_date and get_modification_date helpers
Description of the issue/feature this PR addresses
This PR fixes timestamp filtering and sorting for API queries using
createdandmodifiedfields. It ensures results are filtered and sorted with full second precision, even when catalog metadata does not include these fieldsCurrent behavior before PR
API queries with
created_sinceormodified_sincecould return records with timestamps before the specified threshold. Sorting bycreatedandmodifieddid not work if these fields were missing from catalog metadataDesired behavior after PR is merged
API queries now filter records by the exact timestamp, and sorting by
createdandmodifiedworks reliably using application-side logic. Results are accurate to the second, regardless of catalog metadata configuration.--
I confirm I have tested the PR thoroughly and coded it according to PEP8
standards.