fix(hub): omit default build query parameter

hub/app/controllers/builds_controller.rb

@@ -11,6 +11,7 @@ def index
     load_prototype
     return if performed?
 
+    @history_build = history_build
     before = params[:before]
     before_at = params[:before_at]
     return head :not_found if before_at && !before
@@ -27,11 +28,17 @@ def index
     return head :not_found if before && page_builds.empty?
 
     @builds = page_builds.first(BUILDS_PER_PAGE)
-    @newest_page_path = repository_prototype_builds_path(@repository.route_id, @prototype) if before
+    if before
+      @newest_page_path = repository_prototype_builds_path \
+        @repository.route_id,
+        @prototype,
+        **history_parameters
+    end
     if page_builds.size > BUILDS_PER_PAGE
       @next_page_path = repository_prototype_builds_path \
         @repository.route_id,
         @prototype,
+        **history_parameters,
         before: @builds.last.number,
         before_at: @builds.last.chronological_at.iso8601(6)
     end
@@ -65,7 +72,8 @@ def load_prototype
 
   def prepare_review_header
     @latest_build = @prototype.current_build || @prototype.builds.chronological.first
-    @selected_build = @prototype.builds.where(state: "successful").chronological.first
+    @selected_build = @history_build ||
+      @prototype.builds.where(state: "successful").chronological.first
     @build_options = @prototype.builds.chronological.to_a
     @can_manage_share_links = @prototype.share_links_manageable_by? \
       Current.user,
@@ -90,6 +98,19 @@ def valid_time?(value)
     false
   end
 
+  def history_build
+    number = params[:build]
+    return if number.nil? || (number.is_a?(ActionController::Parameters) && number.empty?)
+
+    raise ActiveRecord::RecordNotFound if !valid_build_number?(number)
+
+    @prototype.builds.where(state: "successful").find_by!(number: number.to_i)
+  end
+
+  def history_parameters
+    @history_build ? { build: @history_build.number } : {}
+  end
+
   # Adds the captured timestamp required by chronological pagination.
   def redirect_legacy_cursor(number)
     return head :not_found if !valid_build_number?(number)
@@ -98,6 +119,7 @@ def redirect_legacy_cursor(number)
     redirect_to repository_prototype_builds_path \
       @repository.route_id,
       @prototype,
+      **history_parameters,
       before: build.number,
       before_at: build.chronological_at.iso8601(6)
   end

hub/app/controllers/concerns/has_share_link_context.rb

@@ -61,9 +61,10 @@ def create_share_link
     build = @prototype.builds.where(state: "successful").find_by! \
       number: positive_build_number(attributes[:build_number])
     if attributes[:expiration] == "never"
+      return_to = Current.canvas_grant ? root_path : management_path
       return if !require_fresh_authorization(
         purpose: "never_expiring_share",
-        return_to: repository_prototype_share_links_path(@repository.route_id, @prototype, build: build.number)
+        return_to:
       )
     end
 

hub/app/controllers/share_links_controller.rb

@@ -6,6 +6,7 @@ class ShareLinksController < SuperglueController
   before_action :load_share_link_context
   before_action :require_public_sharing, only: %i[create destroy]
   before_action :validate_control_parameters, only: [ :create, :destroy ]
+  before_action :validate_management_build, only: %i[create destroy]
 
   rescue_from GithubClient::Error, with: :repository_unavailable
   rescue_from SharingSetting::Disabled, SharingSetting::Changed, with: :public_sharing_disabled
@@ -23,7 +24,7 @@ def create
     share_link = create_share_link
     return if performed?
 
-    redirect_to management_path(share_link.build.number), status: :see_other
+    redirect_to management_path, status: :see_other
   rescue ShareLink::InvalidExpiration, ActiveRecord::RecordInvalid => error
     invalid_share_link(error)
   end
@@ -32,9 +33,8 @@ def create
   # @route DELETE /repositories/:repository_github_id/prototypes/:prototype_id/share_links/:id (repository_prototype_share_link)
   def destroy
     share_link = @prototype.share_links.find(params[:id])
-    build_number = share_link.build.number
     share_link.destroy!
-    redirect_to management_path(build_number), status: :see_other
+    redirect_to management_path, status: :see_other
   end
 
   private
@@ -78,13 +78,21 @@ def selected_build
     builds.find_by!(number: positive_build_number(params[:build]))
   end
 
-  # Returns the management page for the selected Build.
-  def management_path(build_number)
+  # Returns to the same default or explicitly selected management view.
+  def management_path
     repository_prototype_share_links_path \
       @repository.route_id,
       @prototype,
       **@control_parameters,
-      build: build_number
+      **(@management_build_number ? { build: @management_build_number } : {})
+  end
+
+  def validate_management_build
+    value = params[:management_build]
+    return if value.nil?
+
+    number = positive_build_number(value)
+    @management_build_number = @prototype.builds.where(state: "successful").find_by!(number:).number
   end
 
   def validate_control_parameters
@@ -114,7 +122,7 @@ def control_parameters
   def invalid_share_link(error)
     record_security_audit(outcome: "failed")
     redirect_to \
-      management_path(positive_build_number(params.dig(:share_link, :build_number))),
+      management_path,
       alert: share_link_errors(error).to_sentence,
       status: :see_other
   end

hub/app/javascript/features/prototype-workspace-header/stories.tsx

@@ -14,7 +14,7 @@ const data: PrototypeWorkspaceHeaderData = {
     url: "https://github.com/octocat",
   },
   branchName: "prototype/welcome",
-  branchPath: "/repositories/2001/prototypes/1?build=1",
+  branchPath: "/repositories/2001/prototypes/1",
   buildOptions: [
     {
       number: 1,
@@ -35,7 +35,7 @@ const data: PrototypeWorkspaceHeaderData = {
     number: 1,
   },
   shareLinksLabel: "Share links",
-  shareLinksPath: "/repositories/2001/prototypes/1/share_links?build=1",
+  shareLinksPath: "/repositories/2001/prototypes/1/share_links",
   useDarkThemeLabel: "Use dark theme",
   useLightThemeLabel: "Use light theme",
 }

hub/app/javascript/features/share-link-item/index.tsx

@@ -15,6 +15,7 @@ export function ShareLinkItem({
   deleteLabel,
   openLabel,
   controlQuery = "",
+  managementBuildNumber,
 }: ShareLinkItemProps) {
   return (
     <ShareLinkRow
@@ -37,6 +38,9 @@ export function ShareLinkItem({
         <input type="hidden" name="_method" value="delete" />
         <input type="hidden" name="authenticity_token" value={authenticityToken} />
         <input type="hidden" name="control_query" value={controlQuery} />
+        {managementBuildNumber ? (
+          <input type="hidden" name="management_build" value={managementBuildNumber} />
+        ) : null}
         <HubIconButton
           type="submit"
           icon={TrashIcon}

hub/app/javascript/features/share-link-item/types.ts

@@ -9,6 +9,7 @@ export interface ShareLinkData extends ShareLinkDisplayData {
 
 export interface ShareLinkItemProps {
   controlQuery?: string
+  managementBuildNumber?: number
   authenticityToken: string
   shareLink: ShareLinkData
   copiedLabel: string

hub/app/javascript/hooks/use-prototype-viewer-url/index.test.ts

@@ -12,11 +12,7 @@ const selectedBuild = {
 
 describe("usePrototypeViewerUrl()", () => {
   it("refreshes the copied viewer URL when controls change without losing its page destination", () => {
-    window.history.replaceState(
-      {},
-      "",
-      "/repositories/2001/prototypes/1?build=2&return_path=%2Fsettings",
-    )
+    window.history.replaceState({}, "", "/repositories/2001/prototypes/1?return_path=%2Fsettings")
     const { result } = renderHook(() =>
       usePrototypeViewerUrl({ selectedBuild, route: null, initiallyMinimized: false }),
     )
@@ -29,26 +25,28 @@ describe("usePrototypeViewerUrl()", () => {
     const url = new URL(result.current.viewerUrl)
     expect(url.searchParams.get("rc.page.title")).toBe('"Configured"')
     expect(url.searchParams.get("return_path")).toBe("/settings")
+    expect(url.searchParams.has("build")).toBe(false)
     expect(window.location.href).toBe(result.current.viewerUrl)
   })
 
   beforeEach(() => {
     window.history.replaceState(
       { superglue: "saved-page" },
       "",
-      "/repositories/2001/prototypes/1?source=gallery",
+      "/repositories/2001/prototypes/1?source=gallery#hub",
     )
   })
 
-  it("keeps the Hub address and history state while recording the exact Build and start page", () => {
+  it("keeps the default Hub address unpinned while recording the start page", () => {
     const { result } = renderHook(() =>
       usePrototypeViewerUrl({ selectedBuild, route: null, initiallyMinimized: false }),
     )
     const url = new URL(result.current.viewerUrl)
     expect(url.pathname).toBe("/repositories/2001/prototypes/1")
     expect(url.searchParams.get("source")).toBe("gallery")
-    expect(url.searchParams.get("build")).toBe("2")
+    expect(url.searchParams.has("build")).toBe(false)
     expect(url.searchParams.get("return_path")).toBe("/copilot?tab=all#overview")
+    expect(url.hash).toBe("#hub")
     expect(window.location.href).toBe(result.current.viewerUrl)
     expect(window.history.state).toEqual({ superglue: "saved-page" })
   })
@@ -71,8 +69,46 @@ describe("usePrototypeViewerUrl()", () => {
     expect(new URL(result.current.viewerUrl).searchParams.get("return_path")).toBe(
       "/copilot/task/123?filter=a%26b&next=%2Fsettings#/details?tab=activity",
     )
+    expect(new URL(result.current.viewerUrl).searchParams.has("build")).toBe(false)
+    expect(new URL(result.current.viewerUrl).hash).toBe("#hub")
     expect(window.location.href).toBe(result.current.viewerUrl)
     expect(window.history.length).toBe(length)
+    expect(window.history.state).toEqual({ superglue: "saved-page" })
+  })
+
+  it("keeps an explicitly selected latest Build pinned through viewer updates", () => {
+    window.history.replaceState(
+      { superglue: "pinned-page" },
+      "",
+      "/repositories/2001/prototypes/1?build=2&source=gallery#hub",
+    )
+    const initialProps: { route: ReviewRoute | null } = { route: null }
+    const { result, rerender } = renderHook(
+      ({ route }: typeof initialProps) =>
+        usePrototypeViewerUrl({ selectedBuild, route, initiallyMinimized: false }),
+      { initialProps },
+    )
+    expect(new URL(result.current.viewerUrl).searchParams.get("build")).toBe("2")
+
+    rerender({ route: { pathname: "/settings", search: "?tab=profile", hash: "#details" } })
+    act(() => result.current.setMinimized(true))
+    act(() => {
+      const url = new URL(window.location.href)
+      url.searchParams.set("rc.page.title", '"Configured"')
+      window.history.replaceState(window.history.state, "", url)
+      window.dispatchEvent(new Event("replica:controls:url"))
+    })
+    const url = new URL(result.current.viewerUrl)
+    expect(url.searchParams.get("build")).toBe("2")
+    expect(url.searchParams.get("return_path")).toBe("/settings?tab=profile#details")
+    expect(url.searchParams.get("rc.page.title")).toBe('"Configured"')
+    expect(url.searchParams.get("view")).toBe("minimized")
+    expect(url.searchParams.get("source")).toBe("gallery")
+    expect(url.hash).toBe("#hub")
+    expect(window.location.href).toBe(result.current.viewerUrl)
+    expect(window.history.state).toEqual({ superglue: "pinned-page" })
+    act(() => result.current.setMinimized(false))
+    expect(new URL(result.current.viewerUrl).searchParams.get("build")).toBe("2")
   })
 
   it("shares minimized mode and removes only that mode when maximizing", () => {
@@ -89,9 +125,11 @@ describe("usePrototypeViewerUrl()", () => {
     )
     act(() => result.current.setMinimized(true))
     expect(new URL(window.location.href).searchParams.get("view")).toBe("minimized")
+    expect(new URL(result.current.viewerUrl).searchParams.has("build")).toBe(false)
   })
 
   it("keeps the last page while the same iframe reloads but uses the next Build's start page", () => {
+    window.history.replaceState({}, "", "/repositories/2001/prototypes/1?build=2")
     const initialProps: {
       selectedBuild: typeof selectedBuild
       route: ReviewRoute | null
@@ -115,6 +153,49 @@ describe("usePrototypeViewerUrl()", () => {
     expect(new URL(result.current.viewerUrl).searchParams.get("return_path")).toBe("/new")
   })
 
+  it("keeps a reopened default destination when a newer Build is selected", () => {
+    const destination = "/settings?tab=profile#details"
+    window.history.replaceState(
+      {},
+      "",
+      `/repositories/2001/prototypes/1?return_path=${encodeURIComponent(destination)}`,
+    )
+    const { result, rerender } = renderHook(
+      ({ selectedBuild }: { selectedBuild: { number: number; previewUrl: string } }) =>
+        usePrototypeViewerUrl({ selectedBuild, route: null, initiallyMinimized: false }),
+      { initialProps: { selectedBuild } },
+    )
+    rerender({
+      selectedBuild: { number: 3, previewUrl: "http://b13.preview.replica.localhost:19500/new" },
+    })
+    const url = new URL(result.current.viewerUrl)
+    expect(url.searchParams.get("return_path")).toBe(destination)
+    expect(url.searchParams.has("build")).toBe(false)
+  })
+
+  it("keeps a restored history destination unpinned without creating a history entry", () => {
+    const { result } = renderHook(() =>
+      usePrototypeViewerUrl({ selectedBuild, route: null, initiallyMinimized: false }),
+    )
+    const length = window.history.length
+    act(() => {
+      window.history.replaceState(
+        { superglue: "restored-page" },
+        "",
+        "/repositories/2001/prototypes/1?return_path=%2Fsettings%3Ftab%3Dprofile%23details&rc.page.title=%22Restored%22#hub",
+      )
+      window.dispatchEvent(new PopStateEvent("popstate"))
+    })
+    const url = new URL(result.current.viewerUrl)
+    expect(url.searchParams.get("return_path")).toBe("/settings?tab=profile#details")
+    expect(url.searchParams.get("rc.page.title")).toBe('"Restored"')
+    expect(url.searchParams.has("build")).toBe(false)
+    expect(url.hash).toBe("#hub")
+    expect(window.location.href).toBe(result.current.viewerUrl)
+    expect(window.history.state).toEqual({ superglue: "restored-page" })
+    expect(window.history.length).toBe(length)
+  })
+
   it("does not change the browser address without a successful Build", () => {
     const original = window.location.href
     const { result } = renderHook(() =>
@@ -132,7 +213,7 @@ describe("usePrototypeViewerUrl()", () => {
       "/repositories/1/replica?build=99&before=invalid&return_path=%2F%23%2Fabout&view=minimized",
     )
     const { result } = renderHook(() =>
-      usePrototypeViewerUrl({ route: null, initiallyMinimized: true, pinBuild: false }),
+      usePrototypeViewerUrl({ route: null, initiallyMinimized: true, buildSelection: "ignore" }),
     )
     const url = new URL(result.current.viewerUrl)
     expect(result.current.minimized).toBe(false)
@@ -157,7 +238,7 @@ describe("usePrototypeViewerUrl()", () => {
           selectedBuild,
           route,
           initiallyMinimized: false,
-          pinBuild: false,
+          buildSelection: "ignore",
         }),
       { initialProps },
     )
@@ -187,7 +268,7 @@ describe("usePrototypeViewerUrl()", () => {
           selectedBuild,
           route,
           initiallyMinimized: true,
-          pinBuild: false,
+          buildSelection: "ignore",
         }),
       { initialProps: { route: null as ReviewRoute | null } },
     )