Skip to content

Add board settings fields to board JSON responses#2788

Merged
flavorjones merged 4 commits intobasecamp:mainfrom
robzolkos:board-settings-read-parity
Apr 8, 2026
Merged

Add board settings fields to board JSON responses#2788
flavorjones merged 4 commits intobasecamp:mainfrom
robzolkos:board-settings-read-parity

Conversation

@robzolkos
Copy link
Copy Markdown
Collaborator

@robzolkos robzolkos commented Apr 2, 2026

Summary

Add board settings read parity to the board JSON API.

What changed

  • add public_description and public_description_html to the board partial, gated on published? — available on both index and show
  • add user_ids to board show JSON for boards with granular access (all_access: false)
  • add controller tests covering public description fields (published/unpublished), granted user_ids, and omission of user_ids for all-access boards
  • update docs/api/sections/boards.md to document the enriched board response

Notes

  • user_ids matches the existing board update API shape and can be resolved via GET /:account_slug/users
  • public_description fields follow the same pattern as public_url — only present when the board is published
  • user_ids is only present on show, and only when all_access is false

Copy link
Copy Markdown

@claude claude bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

Copilot AI review requested due to automatic review settings April 2, 2026 21: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

This PR adds enriched board JSON responses by including public_description, public_description_html, and user_ids fields in the GET /boards/:id.json endpoint. The changes align with the existing board update API shape and add comprehensive test coverage along with updated documentation.

Changes:

  • Added public_description and public_description_html fields to board show JSON responses
  • Added conditional user_ids field (only present when all_access: false)
  • Added comprehensive controller tests covering the new fields and their conditional inclusion
  • Updated API documentation with example response showing the new fields and conditional field behavior

Reviewed changes

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

File Description
test/controllers/boards_controller_test.rb Added three tests: one for public_description fields, one for user_ids when board is not all access, and one ensuring user_ids is excluded for all-access boards
docs/API.md Updated board show endpoint documentation with new fields in example response and clarified conditional field behavior
app/views/boards/show.json.jbuilder Added public_description, public_description_html, and conditional user_ids fields

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

@robzolkos robzolkos force-pushed the board-settings-read-parity branch from 3df413f to 9a96410 Compare April 6, 2026 19:38
Copilot AI review requested due to automatic review settings April 6, 2026 19:54
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 4 out of 4 changed files in this pull request and generated no new comments.


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

@robzolkos
Copy link
Copy Markdown
Collaborator Author

Re: trix-content class — Yes, Action Text's to_s wraps content in <div class="trix-content">. That's the actual output of board.public_description.to_s, so the doc example is accurate.

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 4 out of 4 changed files in this pull request and generated 1 comment.


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

@robzolkos robzolkos requested a review from flavorjones April 7, 2026 15:11
Copy link
Copy Markdown
Member

@flavorjones flavorjones left a comment

Choose a reason for hiding this comment

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

Suggested a small style change, otherwise this looks good.

Copilot AI review requested due to automatic review settings April 8, 2026 12:26
@flavorjones flavorjones merged commit 2a4adf3 into basecamp:main Apr 8, 2026
10 checks passed
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 4 out of 4 changed files in this pull request and generated 3 comments.


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

Comment on lines 1 to +2
json.partial! "boards/board", board: @board
json.user_ids @board.users.ids unless @board.all_access?
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

user_ids is derived from board access records, but BoardsController#show uses conditional GET (fresh_when) with an ETag that doesn’t incorporate access changes. Because board access grants use Access.insert_all (bypassing the touch: true on Access#board), the board’s cache key/ETag can remain unchanged after membership updates, causing stale user_ids responses (304 Not Modified) for clients. Consider either touching the board when revising accesses, or including an access-based version (e.g., @board.accesses.maximum(:updated_at) / count) in the show ETag when rendering JSON with user_ids.

Copilot uses AI. Check for mistakes.
@@ -1 +1,2 @@
json.partial! "boards/board", board: @board
json.user_ids @board.users.ids unless @board.all_access?
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

@board.users.ids has no explicit ordering (the association is unordered), so the JSON array order can be nondeterministic across DBs/queries. For a stable API response (and to avoid clients seeing spurious diffs), consider returning a deterministic order (e.g., by ordering in SQL before plucking IDs).

Suggested change
json.user_ids @board.users.ids unless @board.all_access?
json.user_ids @board.users.order(:id).ids unless @board.all_access?

Copilot uses AI. Check for mistakes.
Comment on lines +9 to +12
if board.published?
json.public_description board.public_description.to_plain_text
json.public_description_html board.public_description.to_s
json.public_url published_board_url(board)
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

This partial now calls board.published? and renders board.public_description (ActionText) for published boards. BoardsController#index only eager-loads creator: :identity, so on a cold fragment cache this will introduce per-board queries for publication and rich_text_public_description (N+1) when listing boards as JSON. Consider eager-loading these associations in the index query (e.g., include :publication and the rich text association) to keep index response query counts stable.

Copilot uses AI. Check for mistakes.
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.

3 participants