Add usage and response counts to get guestbooks list#12269
Add usage and response counts to get guestbooks list#12269stevenwinship wants to merge 2 commits intodevelopfrom
Conversation
This comment has been minimized.
This comment has been minimized.
1 similar comment
|
📦 Pushed preview images as 🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name. |
scolapasta
left a comment
There was a problem hiding this comment.
Left one comment about the default for discussion.
Also branch is out of date.
| JsonArrayBuilder guestbookArray = Json.createArrayBuilder(); | ||
| JsonPrinter jsonPrinter = new JsonPrinter(); | ||
| for (Guestbook gb : guestbooks) { | ||
| // default is to include the stats. Ignore the stats for a faster reply if they are not needed |
There was a problem hiding this comment.
Is there a reason that the default is to include the stats? generally, faster defaults are "better" (for some definition of that word) and then you add the parameter if you need the slower.
If we do change the parameter should naturally be "includeStats". (I would even think we could consider the name change if we don't, partially in case we someday decide to change the default, the name works better for both cases, imo)
There was a problem hiding this comment.
Since JSF includes the stats by default I wanted the API to match. Most of the time the stats are desired. I just added a way to exclude them if someone didn't want them or just wanted a faster listing
There was a problem hiding this comment.
I could just leave it out and always include the stats
What this PR does / why we need it:Currently, the GET Guestbook API returns metadata about guestbooks (e.g., name, required fields, custom questions), but it does not include usage and responses, needed for Manage Guestbooks table
Which issue(s) this PR closes:#12260
Special notes for your reviewer:
Suggestions on how to test this:
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Is there a release notes update needed for this change?: Included
Additional documentation: