Skip to content

23971 Proposed API and schema changes#25013

Merged
jacobshandling merged 6 commits intomainfrom
23971-api-changes
Mar 10, 2025
Merged

23971 Proposed API and schema changes#25013
jacobshandling merged 6 commits intomainfrom
23971-api-changes

Conversation

@jacobshandling
Copy link
Copy Markdown
Contributor

@jacobshandling jacobshandling commented Dec 26, 2024

For #25034

API changes:

this PR diff ("available_teams" change is adding missing documentation for current API behavior)

schema changes:

  • new col in users table, settings, type json. Defaults to {}. New setting, hidden_host_columns, added or updated on first relevant API call per user.

semantics

  • null "hidden_host_columns" field means "not yet set, use defaults": {"settings":{"hidden_host_columns": null}}
  • included and empty "hidden_host_columns" field means "no columns hidden, show all columns in the UI": {"settings":{"hidden_host_columns": []}}

Updates 1/7/25 per discussion with @rachaelshaw @lucasmrod @sgress454:

  • Optional query param include_ui_settings=true included with GETs to /me or /users/:id will trigger considering the API call to be a contributor API call, giving more flexibility for future changes. Note that this is the first time we have one endpoint that can be conditionally considered a contributor endpoint depending on how it is called.

Comment thread docs/REST API/rest-api.md Outdated
@jacobshandling jacobshandling changed the title 23971 Proposed API changes 23971 Proposed API and schema changes Dec 26, 2024
lucasmrod
lucasmrod previously approved these changes Dec 26, 2024
Copy link
Copy Markdown
Contributor

@iansltx iansltx left a comment

Choose a reason for hiding this comment

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

Was going to ask whether we should revise the default on me?include_settings but it looks like loading the hosts page pulls /me anyway so we can just revise that call to pull column settings when we need them (vs. trying to economize on HTTP requests now).

Comment thread docs/REST API/rest-api.md Outdated
lucasmrod
lucasmrod previously approved these changes Dec 30, 2024
@jacobshandling
Copy link
Copy Markdown
Contributor Author

Hey @rachaelshaw, hope you had a nice vacation. Assigning you this ticket, how do these specs look from product POV?

Copy link
Copy Markdown
Member

@rachaelshaw rachaelshaw left a comment

Choose a reason for hiding this comment

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

Approach lgtm, left a few small comments

Comment thread docs/REST API/rest-api.md Outdated
Comment thread docs/REST API/rest-api.md Outdated
Comment thread docs/REST API/rest-api.md Outdated
Comment thread docs/REST API/rest-api.md Outdated
Comment thread docs/REST API/rest-api.md Outdated
@jacobshandling
Copy link
Copy Markdown
Contributor Author

@rachaelshaw @lucasmrod @sgress454 added our decisions to this issue description, thanks for your insights!

lucasmrod
lucasmrod previously approved these changes Jan 8, 2025
@lucasmrod
Copy link
Copy Markdown
Member

@jacobshandling Should this PR be merged or closed? (IIRC the functionality was merged to main).

@jacobshandling
Copy link
Copy Markdown
Contributor Author

@lucasmrod I'm unsure of doc update process re closing older PRs and opening new ones to merge, so was leaving this info for @rachaelshaw's reference. Rachael, should this PR be closed?

@lucasmrod lucasmrod removed their assignment Jan 30, 2025
@jacobshandling jacobshandling marked this pull request as ready for review March 10, 2025 17:17
@jacobshandling jacobshandling merged commit 1268036 into main Mar 10, 2025
@jacobshandling jacobshandling deleted the 23971-api-changes branch March 10, 2025 17:18
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.

5 participants