Add support for paginating elasticsearch queries for django_tables2 (with HTMX)
#35712
Conversation
this will be used in other areas to maintain consistency with case list explorer report sorting
this ElasticRecord will support only CaseSearchES for now, as that's what we will need in the near future for data cleaning cases
biyeun
requested review from
millerdev,
mjriley,
orangejenny,
esoergel,
jingcheng16 and
nospame
biyeun
requested review from
MartinRiese,
AddisonDunn,
stephherbers,
Robert-Costello and
Jtang-1
as code owners
|
|
||
| @property | ||
| @abstractmethod | ||
| def verbose_name_plural(self): |
There was a problem hiding this comment.
Perhaps the default implementation could just add a 's' to the verbose_name.
There was a problem hiding this comment.
i'm following requirements of the base models from django_tables2, and that's not always the case with translated text that plurals end in s, in english or other languages
There was a problem hiding this comment.
Emphasis on default. But ok.
| def __init__(self, **kwargs): | ||
| data = kwargs.pop('data') | ||
| if not ElasticTableData.validate(data): | ||
| raise ValueError( |
| if isinstance(original_exception, TransportError): | ||
| if hasattr(original_exception.info, "get"): | ||
| if original_exception.info.get('status') == 400: | ||
| raise BadRequestError() |
There was a problem hiding this comment.
Does TransportError have any other data in it about what went wrong? I don't like the idea of erasing that and potentially only logging a BadRequestError, which does not give you much to work with when investigating an issue.
There was a problem hiding this comment.
i'm following the pattern we use with CaseListExplorer for fetching results
There was a problem hiding this comment.
That does not mean we cannot improve on it.
| if isinstance(original_exception, TransportError): | ||
| if hasattr(original_exception.info, "get"): | ||
| if original_exception.info.get('status') == 400: | ||
| raise BadRequestError() |
There was a problem hiding this comment.
That does not mean we cannot improve on it.
Technical Summary
This adds support for paginating elasticsearch queries with
django_tables2with the introduction of theElasticTableclass that hooks into theElasticTableDataclass that handles the ordering and pagination of the query based on theElasticTable'srecord_classpassed to it.The first
ElasticRecordimplementation is forCaseSearchESwithCaseSearchElasticRecord. From this example, you can see how easy it would be to extend this support for other queries, likeFormESSee this commit for how we would create a simple
HTMX+django_tables2view with cases fromCaseSearchES: badb29dSafety Assurance
Safety story
There is no production workflow using these changes yet. The bit of code that was extracted from
CaseListExplorershould be very easy to identify that it's safe without needing QA. As a sanity check I made sureCaseListExplorersorted on@case_idwith no issue.Automated test coverage
I've added new tests.
QA Plan
Not needed
Rollback instructions
Labels & Review