Link version navigation by slug so platformed variants resolve - #871
Open
jenshenny wants to merge 1 commit into
Open
Link version navigation by slug so platformed variants resolve#871jenshenny wants to merge 1 commit into
jenshenny wants to merge 1 commit into
Conversation
The previous/next links on the version page were built from the version number alone, which resolves the full_name name-number — only ever the plain ruby variant. When the adjacent number exists only as platformed or content-addressable versions the link 404ed. Link by slug, which encodes the platform or content address, and make the adjacent-position lookup deterministic since variants of a number share a position.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
rubygems#6674
Link version navigation by slug so platformed variants resolve
Problem
The previous/next links on the version show page are built from the version number alone:
That path resolves via
full_name = "name-number"— only ever the plain-ruby variant. When the adjacent number exists only as platformed versions, the link has always 404ed (pre-existing bug); content-addressable variants make this much more likely, since every skinny gem's slug carries its content address.Version#previous/#nextalso usedfind_by(position: ±1)— andpositionis ranked by number alone, so all variants of a number share a position and the lookup returned an arbitrary row.Fix
previous.slug/next.slug— the slug encodes the platform or content address, so every variant shape resolvesorder(:id).first)Testing
Integration test builds a gem with a plain version, a platformed version, and a content-addressable version, then walks the navigation in both directions asserting the exact hrefs (scoped to the nav link classes) and following them to a 200. Verified the test fails without the fix (the next-link href pointed at the bare number and 404ed).
Found during the content-addressable deep review; pre-existing on master but surfaced by ABI variants, hence targeting the feature branch where the test can cover the content-addressed case.