-
-
Notifications
You must be signed in to change notification settings - Fork 359
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
Change response structure for record summary #3802
Conversation
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.
This is returning results like:
{
"2": [
{
"1234": "Foo bar"
},
{
"5678": "Baz bat"
}
]
}
Instead, I want results like:
{
"2": {
"1234": "Foo bar",
"5678": "Baz bat"
}
}
841c842
to
b214094
Compare
Ahh, should be now @seancolsen. |
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. This gives me what I need now 🙂
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.
One small typing issue.
If it's not a problem for @seancolsen , please just fix the type annotation and merge. If it is, we need to rethink the response form since PostgreSQL can't do non-text JSON keys.
I'm going to approve (provisionally) since in the former case it's a small change and I don't want to hold things up.
mathesar/rpc/records.py
Outdated
""" | ||
count: int | ||
results: list[dict] | ||
grouping: GroupingResponse | ||
preview_data: dict[str, list[PreviewEntry]] | ||
preview_data: dict[str, dict[Any, str]] |
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.
The key will always be a string due to the nature of jsonb_object_agg
. So, this should be:
preview_data: dict[str, dict[Any, str]] | |
preview_data: dict[str, dict[str, str]] |
@seancolsen To double-check, are you sure that mistyping won't cause a problem?
mathesar/rpc/records.py
Outdated
""" | ||
results: list[dict] | ||
preview_data: dict[str, list[PreviewEntry]] | ||
preview_data: dict[str, dict[Any, str]] |
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.
Same
preview_data: dict[str, dict[Any, str]] | |
preview_data: dict[str, dict[str, str]] |
@@ -64,7 +64,7 @@ def mock_list_records( | |||
{"id": 3, "count": 8, "results_eq": {"1": "lsfj", "2": 3422}} | |||
] | |||
}, | |||
"preview_data": {"2": [{"key": 12345, "summary": "blkjdfslkj"}]}, | |||
"preview_data": {"2": {"12345": "blkjdfslkj"}}, |
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.
The issue I mentioned above is visible here.
@seancolsen I re-requested review from you to check that the typing issue I mentioned won't be a problem. |
@mathemancer @Anish9901 a few thoughts in response to the recent comments on this PR:
|
Fixes #3801
Checklist
Update index.md
).develop
branch of the repositoryvisible errors.
Developer Certificate of Origin
Developer Certificate of Origin