Skip to content

refactor(bounty-linking): extract hooks for proposal-view reuse - #1699

Closed
jwalkingjew wants to merge 5 commits into
masterfrom
feat/link-bounty-on-proposal-view
Closed

jwalkingjew wants to merge 5 commits into
masterfrom
feat/link-bounty-on-proposal-view

Conversation

@jwalkingjew

@jwalkingjew jwalkingjew commented Apr 21, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Behavior-preserving refactor that pulls the bounty fetch/merge and "Bounty links for: …" publish logic out of review-changes.tsx into two reusable hooks:

  • useLinkableBounties — fetch eligible bounties (ancestor-space search, submission counts, space labels) and merge with locally-edited bounties from the sync store.
  • usePublishBountyLinks — publish a Proposal entity + Bounties relations into the author's personal space.

This is prep work for [plan item: link bounties from the proposal voting screen]. On the review screen, behavior is unchanged.

Follow-ups on this branch

  • useLinkedBountiesForProposal (read currently-linked bounties)
  • Design refresh on BountyCard + BountyLinkingPanel per Figma
  • Wire author-gated linking UI into active-proposal.tsx

Test plan

  • Review screen still lists the same eligible bounties as before
  • Linking a bounty at publish time still creates the "Bounty links for: …" proposal in the personal space
  • bun run build passes

…ountyLinks hooks

Pulls the bounty fetch-and-merge logic and the "bounty links for: …" publish
logic out of review-changes.tsx so the proposal voting screen can reuse the
same machinery for post-hoc linking. Behavior is unchanged on the review
screen.
@vercel

vercel Bot commented Apr 21, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
geogenesis Ready Ready Preview Apr 21, 2026 7:24pm

Request Review

Copilot AI left a comment

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.

Pull request overview

Refactors the review screen’s bounty-linking logic into reusable hooks so the same bounty fetch/merge and “Bounty links for: …” publishing behavior can be reused from other proposal UIs (e.g. voting screen) without changing current review behavior.

Changes:

  • Extracted eligible bounty discovery/merge + space label enrichment into useLinkableBounties.
  • Extracted “publish bounty links proposal” logic into usePublishBountyLinks.
  • Updated review-changes.tsx to consume the new hooks and simplified inline logic.

Reviewed changes

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

File Description
apps/web/partials/review/review-changes.tsx Replaces inline bounty fetch/merge + publish logic with the new reusable hooks.
apps/web/partials/review/bounty-linking/use-linkable-bounties.ts New hook encapsulating linkable bounty querying, merge with local store edits, submission counts, and space label/image enrichment.
apps/web/partials/review/bounty-linking/use-publish-bounty-links.ts New hook encapsulating publishing a “Bounty links for: …” proposal + relations into personal space.
apps/web/partials/review/bounty-linking/index.ts Re-exports the new hooks from the bounty-linking barrel.

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

Comment on lines +223 to +229
for (const id of bountySpaceIdsForLabels) {
const space = bountyLabelSpaces.find(s => s.id === id);
const name = space?.entity?.name?.trim();
const label = name && name.length > 0 ? name : bountySpaceFallbackLabel(id);
const image =
space?.entity?.image && space.entity.image.length > 0 ? space.entity.image : PLACEHOLDER_SPACE_IMAGE;
row.set(id, { label, image });

Copilot AI Apr 21, 2026

Copy link

Choose a reason for hiding this comment

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

Building the row map does an Array.find over bountyLabelSpaces for each space id, which is O(n²) in the number of spaces. Consider precomputing a Map from bountyLabelSpaces keyed by id, then doing O(1) lookups inside the loop.

Copilot uses AI. Check for mistakes.
Comment on lines +45 to +53
const { makeProposal } = usePublish();
const [isPublishing, setIsPublishing] = React.useState(false);

const publish = React.useCallback(
async ({ proposalId, proposalName, toSpaceId, bountyIds, bountiesById, onSuccess, onError }: PublishBountyLinksArgs) => {
if (!personalSpaceId) {
onError?.();
return;
}

Copilot AI Apr 21, 2026

Copy link

Choose a reason for hiding this comment

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

usePublish().makeProposal can return early without calling onSuccess/onError (e.g. when smartAccount is missing). In that case this hook will silently no-op and callers won't get an error callback. Consider capturing the return from makeProposal(...) and, if it's falsy, invoke onError (and/or return a rejected promise) so consumers can handle the failure deterministically.

Copilot uses AI. Check for mistakes.
Comment on lines +248 to +252
return {
bounties: bountiesWithSpaceLabels,
bountiesById: bountiesByIdWithLabels,
isLoading: isLoadingAncestors || isLoadingRemote,
};

Copilot AI Apr 21, 2026

Copy link

Choose a reason for hiding this comment

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

The returned isLoading only reflects the ancestor-space and remote-bounty queries, but this hook also fetches submission relations and space labels (which affect submissionsCount/userSubmissionsCount and spaceLabel/spaceImage). Either include those query loading states in isLoading, or rename the flag to clarify what it actually represents so future consumers don't treat the data as fully hydrated too early.

Copilot uses AI. Check for mistakes.
- Precompute the space-label map so bountiesWithSpaceLabels is O(n) instead
  of O(n*m) on every render.
- Include submission/label query loading states in the returned isLoading
  so consumers can't treat partially-hydrated bounties as ready.
- Surface makeProposal's silent early-return paths (missing smart account,
  missing space, empty ops) as onError calls so publish callers get a
  deterministic failure signal.
Adds post-hoc bounty linking so a proposal's author can attach bounties
from the governance view during voting or after a proposal has been
accepted/executed/rejected — not just at proposal-creation time.

- New useLinkedBountiesForProposal hook reads the Proposal entity's
  outgoing BOUNTIES_RELATION_TYPE relations and hydrates the bounty cards.
- ProposalLinkedBountiesList shows the linked bounties (read-only) to all
  viewers of a proposal; ProposalBountyLinksLauncher shows a 'Link to
  bounty' button + slide-in picker to the author only.
- BountyCard gains a readOnly prop for the read-only list, relabels budget
  to 'Max payout' / 'Est. payout' based on status, drops the redundant
  Submissions row, and renders 'Unlimited' when no per-person cap is set.

MVP is add-only: post-hoc removal of already-linked bounties is deferred.
@jwalkingjew
jwalkingjew marked this pull request as ready for review April 21, 2026 18:02
…rt unlink

Addresses review feedback on the proposal-view linking experience:

- Linked bounties now live in a right-side slide-in panel, matching the
  review-your-edits layout, instead of an inline block below the voting
  bar. A single header button ('N' or 'Link to bounty') opens the panel
  for viewers and authors alike.
- Author detection moves client-side via useSmartAccount so the button
  and edit affordances appear as soon as the wallet hydrates — previously
  the server-side WALLET_ADDRESS cookie could be unset and the button
  would not render for the actual author.
- Authors can now remove previously-linked bounties by unchecking them in
  the panel and saving. The publish step emits deleteRelation ops for the
  tombstoned Bounties relations on the Proposal entity in the author's
  personal space. Add + remove can happen in the same save; delete-only
  saves skip re-asserting the proposal name value and Types relation.
- useLinkedBountiesForProposal now surfaces the original Relation per
  linked bounty so the unlink path has the ids it needs.
- Fetch personalPageEntityId via useGeoProfile (like review-changes does).
  Previously passed null, so isAllocatedToUser only matched bounties
  allocated to the personal space, hiding bounties allocated to the
  user's page entity. That caused unlinked bounties to disappear from
  the picker and be unable to be re-linked.
- Reuse BountyLinkingPanel directly instead of a lookalike so the card
  chrome, header label ('N bounties linked'), and selection UX stay in
  lockstep with the review-your-edits screen.
- Add readOnly prop to BountyLinkingPanel; forwarded to BountyCard for
  non-author viewers so the checkbox doesn't appear.
@ohohoreilly

Copy link
Copy Markdown
Contributor

Closing as part of a sweep of the open-PR queue. Not a judgement on the work — reopen if you still want it and I will help get it current.

Opened 2026-04-21 and now conflicting with master. At this distance a rebase is usually more work than redoing the change against current code, and the surrounding code has moved a long way underneath "refactor(bounty-linking): extract hooks for proposal-view reuse".

@jwalkingjew — if the idea still stands but the branch does not, a fresh PR or a ticket is probably a better route than reviving this one.

Nothing is discarded: the branch and its history remain, and reopening costs a click.

Context: 74 PRs were open, 25 older than two months, the oldest from February. The point is to make the queue mean something so genuinely ready work is visible rather than buried — #2449 sat ready for three days this week partly because of the noise. Only non-draft, conflicting PRs are in scope; drafts and anything still mergeable are being left alone.

This branch was successfully deployed

1 active deployment
Preview — 16364584 Deployed Apr 21, 2026 by vercel[bot]
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