Skip to content

Add redirect uri list to dev app response#740

Merged
rickyrombo merged 2 commits intomainfrom
mjp-redirect-uri-on-dev-apps
Mar 27, 2026
Merged

Add redirect uri list to dev app response#740
rickyrombo merged 2 commits intomainfrom
mjp-redirect-uri-on-dev-apps

Conversation

@rickyrombo
Copy link
Copy Markdown
Contributor

@rickyrombo rickyrombo commented Mar 26, 2026

  • Understand the codebase and existing tests
  • Extend TestGetDeveloperAppsQueries to assert RedirectUris is an empty slice (not nil)
  • Extend TestGetDeveloperApp to assert redirect_uris is an empty slice (not nil) in the JSON response
  • Extend TestV1UsersDeveloperApps to assert redirect_uris is a JSON array (not null) with length 0 for apps without URIs
  • Add TestV1UsersDeveloperAppsRedirectUrisOrdering: verifies URIs are ordered by oauth_redirect_uris.id (not alphabetically), and that apps without URIs return [] not null

💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.

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

This PR updates the developer app retrieval query to include the list of registered OAuth redirect URIs in the developer app response payload.

Changes:

  • Extend GetDeveloperApps SQL to LEFT JOIN oauth_redirect_uris and aggregate redirect_uris per app.
  • Regenerate sqlc output to add RedirectUris []string to GetDeveloperAppsRow and scan it from the query.
  • Minor whitespace cleanup in api/v1_oauth.go.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
api/v1_oauth.go Removes an extra whitespace-only line (no functional change).
api/dbv1/queries/get_developer_apps.sql Adds redirect_uris aggregation via join to oauth_redirect_uris.
api/dbv1/get_developer_apps.sql.go sqlc regen: includes RedirectUris field and scans it from query results.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +6 to +10
da.description,
da.image_url,
COALESCE(ARRAY_AGG(oau.redirect_uri ORDER BY oau.id) FILTER (WHERE oau.redirect_uri IS NOT NULL), ARRAY[]::text[])::text[] AS redirect_uris
FROM developer_apps da
LEFT JOIN oauth_redirect_uris oau ON oau.client_id = da.address
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

New redirect_uris aggregation is now part of the GetDeveloperApps result set, but there’s no test coverage exercising this behavior (e.g., when URIs exist and when none exist). Please extend the existing developer app query/API tests to assert redirect_uris is returned (and is an empty array rather than null) and that ordering is deterministic (by oauth_redirect_uris.id).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Added test coverage in e70017c:

  • TestGetDeveloperAppsQueries: asserts RedirectUris equals []string{} (not nil) at the query layer when no URIs are registered
  • TestGetDeveloperApp: asserts the deserialized RedirectUris field is an empty slice (not nil) from the /v1/developer_apps/:address endpoint
  • TestV1UsersDeveloperApps: uses gjson.IsArray() to assert redirect_uris is a JSON array (not null) with length 0 for each app without registered URIs
  • TestV1UsersDeveloperAppsRedirectUrisOrdering (new): inserts URIs with z before a alphabetically but in ascending id order, then asserts the response matches insertion order — confirming ordering by oauth_redirect_uris.id, not alphabetically. Also asserts the second app (no URIs) returns [] not null.

@rickyrombo rickyrombo merged commit 33c4e54 into main Mar 27, 2026
5 checks passed
@rickyrombo rickyrombo deleted the mjp-redirect-uri-on-dev-apps branch March 27, 2026 02:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants