Skip to content

CROSSLINK-274 patron request facets#615

Closed
adamdickmeiss wants to merge 36 commits into
mainfrom
CROSSLINK-274-patron-request-facets
Closed

CROSSLINK-274 patron request facets#615
adamdickmeiss wants to merge 36 commits into
mainfrom
CROSSLINK-274-patron-request-facets

Conversation

@adamdickmeiss
Copy link
Copy Markdown
Contributor

@adamdickmeiss adamdickmeiss commented May 26, 2026

https://index-data.atlassian.net/browse/CROSSLINK-274

response in About. Should do facets for any patron_requests query.

Copilot AI review requested due to automatic review settings May 26, 2026 15:16
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds faceting support to the broker’s /patron_requests listing endpoint (CROSSLINK-274), exposing aggregated counts (e.g., by requester/supplier symbol) alongside the normal paginated results.

Changes:

  • Extend the OpenAPI spec and API handler to accept a facets query parameter and return facet data in about.facets.
  • Add SQL + repository/query-layer support to compute facet counts (with optional CQL filtering).
  • Add API tests covering successful facet retrieval and bad facet input.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
broker/test/patron_request/api/api-handler_test.go Adds tests for facets success and invalid facet handling.
broker/sqlc/pr_query.sql Adds a SQLC query to compute facet counts.
broker/patron_request/db/prrepo.go Introduces facet models and repo method to compute facets.
broker/patron_request/db/prcql.go Adds CQL-aware facets query execution with dynamic SQL construction.
broker/patron_request/api/api-handler.go Wires facets into /patron_requests response and adds toProAbout conversion helper.
broker/oapi/open-api.yaml Adds facets parameter and schemas for facet results in About.
Comments suppressed due to low confidence (1)

broker/oapi/open-api.yaml:1422

  • The /patron_requests GET 200 response description says "Successful retrieval of events", which is misleading for API consumers. It should describe patron request retrieval to match the endpoint.
        '200':
          description: Successful retrieval of events
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/PatronRequests'

Comment thread broker/sqlc/pr_query.sql
Comment thread broker/sqlc/pr_query.sql Outdated
Comment thread broker/patron_request/db/prcql.go Outdated
Comment thread broker/patron_request/db/prcql.go Outdated
Comment thread broker/patron_request/db/prrepo.go Outdated
Comment thread broker/patron_request/api/api-handler.go
Comment thread broker/test/patron_request/api/api-handler_test.go Outdated
Copilot AI review requested due to automatic review settings May 26, 2026 15:37
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.

Comments suppressed due to low confidence (1)

broker/oapi/open-api.yaml:1487

  • The /patron_requests GET response description says "Successful retrieval of events", which doesn’t match the endpoint and can confuse API consumers. Consider updating it to "Successful retrieval of patron requests" (or similar).
        - $ref: '#/components/parameters/Limit'
        - $ref: '#/components/parameters/Offset'
        - $ref: '#/components/parameters/Side'
        - $ref: '#/components/parameters/Symbol'
        - $ref: '#/components/parameters/Facets'
      responses:
        '200':
          description: Successful retrieval of events
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/PatronRequests'

Comment thread broker/patron_request/api/api-handler.go
Comment thread broker/patron_request/db/prrepo.go Outdated
Comment thread broker/patron_request/db/prcql.go Outdated
Comment thread broker/patron_request/db/prcql.go Outdated
Comment thread broker/sqlc/pr_query.sql
Comment thread broker/test/patron_request/api/api-handler_test.go Outdated
Comment thread broker/test/patron_request/api/api-handler_test.go Outdated
Comment thread broker/patron_request/db/prrepo.go
Copilot AI review requested due to automatic review settings May 26, 2026 15:59
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

Comments suppressed due to low confidence (1)

broker/oapi/open-api.yaml:1484

  • The response description under GET /patron_requests says “Successful retrieval of events”, which looks like a copy/paste error and makes the OpenAPI misleading for clients. Update it to describe patron requests instead.
        - $ref: '#/components/parameters/Facets'
      responses:
        '200':
          description: Successful retrieval of events
          content:

Comment thread broker/patron_request/db/prrepo.go Outdated
Comment thread broker/patron_request/db/prrepo.go Outdated
Comment thread broker/sqlc/pr_query.sql Outdated
Comment thread broker/patron_request/api/api-handler.go Outdated
Comment thread broker/patron_request/api/api-handler.go Outdated
Comment thread broker/patron_request/db/prcql.go Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 26, 2026 17:02
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 9 comments.

Comment thread broker/patron_request/api/api-handler.go Outdated
Comment thread broker/patron_request/api/api-handler.go Outdated
Comment thread broker/patron_request/db/prrepo.go Outdated
Comment thread broker/patron_request/db/prcql.go
Comment thread broker/sqlc/pr_query.sql Outdated
Comment thread broker/oapi/open-api.yaml Outdated
Comment thread broker/test/patron_request/api/api-handler_test.go Outdated
Comment thread broker/test/patron_request/api/api-handler_test.go Outdated
Comment thread broker/patron_request/db/prrepo.go Outdated
Copilot AI review requested due to automatic review settings May 26, 2026 17:14
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Comment thread broker/patron_request/api/api-handler.go Outdated
Comment thread broker/patron_request/db/prrepo.go Outdated
Comment thread broker/test/patron_request/api/api-handler_test.go
Copilot AI review requested due to automatic review settings May 27, 2026 09:23
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Comment thread broker/patron_request/api/api-handler.go
Comment thread broker/patron_request/db/prrepo.go
Copilot AI review requested due to automatic review settings May 27, 2026 09:53
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Comment thread broker/oapi/open-api.yaml Outdated
Comment thread broker/oapi/open-api.yaml
Comment thread broker/patron_request/db/prrepo.go
Copilot AI review requested due to automatic review settings May 27, 2026 10:56
adamdickmeiss and others added 3 commits May 27, 2026 12:57
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Comment thread broker/patron_request/db/prrepo.go Outdated
Comment thread broker/oapi/open-api.yaml
Copilot AI review requested due to automatic review settings May 27, 2026 11:06
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Comment thread broker/patron_request/db/prrepo.go Outdated
Comment thread broker/patron_request/api/api-handler.go Outdated
Now that the OpenAPI spec is validated. In fact , one could argue
that it an error now should be seena as Internal Error.
Copilot AI review requested due to automatic review settings May 27, 2026 11:24
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

@adamdickmeiss adamdickmeiss requested a review from jakub-id May 27, 2026 11:30
@adamdickmeiss
Copy link
Copy Markdown
Contributor Author

Merged in #617 already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants