Exclude yanked crates on user/team pages #1642

bryanburgers · 2019-02-28T22:00:22Z

Add a URL parameter to the /api/v1/crates endpoint that specifies whether or not the endpoint should return crates that have been fully yanked - i.e., where every version has been yanked.

If not specified, this defaults to true because we have typically always returned all matching crates, whether or not their versions have been yanked.

This makes it possible to display only non-yanked crates at various places in the UI without changing Rust code.

Use this from the user and team pages to exclude yanked crates from them.

Closes #958

sgrif · 2019-03-01T02:40:05Z

app/routes/user.js

@@ -15,6 +15,7 @@ export default Route.extend({
        return this.store.queryRecord('user', { user_id }).then(
            user => {
                params.user_id = user.get('id');
+                params.include_yanked = 'n';


Should we just make this the default and not specify it in the URL here?

Here was my thinking. If it's not the direction we want to go, I can make changes.

There are situations where we want to show yanked crates: #958 (comment), #145 (comment).

Given the following two URLs:

/api/v1/crates?search=foo

/api/v1/crates?user_id=2576

One option would be to add conditional logic to the controller like "if user_id is set, then don't show yanked crates; if search is set, then show yanked crates".

I shied away from that option because I think it would be confusing. A contributor focused on the front end would have to keep in their head which rules apply to which parameters.

Given the following two URLs:

/api/v1/crates?search=foo&include_yanked=yes

/api/v1/crates?user_id=2576&include_yanked=no

It's much less surprising what will be returned from these endpoints.

However, I'm not sure we want to audit and edit every location where the front end calls the /api/v1/crates endpoint – and we can't audit third-party crawlers. Beacuse /api/v1/crates has historically defaulted to showing yanked crates, I opted to only add &include_yanked=no to the specific front end code where we don't want to show yanked crates.

/api/v1/crates?search=foo

/api/v1/crates?user_id=2576&include_yanked=no

This makes the URLs for the API endpoint a little bit more ugly for these pages. However, this largely only shows up in developer tools. This change doesn't add a query parameter to the URL for a user/team page, e.g. a user page continues to be /users/sgrif, not ~~/users/sgrif?include_yanked=no~~.

Am I making any sense? Hopefully I didn't answer a question you weren't asking!

src/controllers/krate/search.rs

bryanburgers · 2019-03-01T14:03:55Z

Note: CI failures are because it's after 2019-02-28 and #1630 hasn't landed yet.

Edit: Rebased after that PR landed.

bors · 2019-03-20T04:11:15Z

☔ The latest upstream changes (presumably #1668) made this pull request unmergeable. Please resolve the merge conflicts.

bors · 2019-04-24T00:32:44Z

☔ The latest upstream changes (presumably #1560) made this pull request unmergeable. Please resolve the merge conflicts.

sgrif · 2019-05-22T17:10:56Z

Just noting for the future -- this causes a roughly 30% performance regression on search, which brings us up to 265ms in the worst case (search for 's' ordered by relevance). I don't think it needs to be addressed in this PR, but I think we're pushing the limits on what's acceptable performance-wise.

We probably want to start thinking about introducing a materialized view that includes this yanked calculation and has already done the join on recent_crate_downloads, which can bring us to 160ms in the worst case -- or we may just want to stop doing the like check on single letter searches, since this will only continue to deteriorate in the future.

@bors: r+

bryanburgers · 2019-05-22T17:29:43Z

Yikes. 30% is bad. I'm concerned about that. But we only pay that 30% when it's used, right? And right now, it won't get used on the search page?

sgrif · 2019-05-22T17:30:52Z

Ah you're right, I missed that this only affects the user page by default

sgrif · 2019-05-22T17:31:19Z

@bors r+

sgrif · 2019-05-22T17:32:55Z

Looks like GH is having an outage that's breaking bors right now

bors · 2019-05-22T18:50:05Z

📌 Commit 4be4d4d has been approved by sgrif

bors · 2019-05-22T18:56:36Z

📌 Commit 4be4d4d has been approved by sgrif

bors · 2019-05-22T19:25:57Z

⌛ Testing commit 4be4d4d with merge 3bf715c...

…-pages, r=sgrif Exclude yanked crates on user/team pages Add a URL parameter to the `/api/v1/crates` endpoint that specifies whether or not the endpoint should return crates that have been fully yanked - i.e., where every version has been yanked. If not specified, this defaults to `true` because we have typically always returned all matching crates, whether or not their versions have been yanked. This makes it possible to display only non-yanked crates at various places in the UI without changing Rust code. Use this from the user and team pages to exclude yanked crates from them. Closes #958

bors · 2019-05-22T19:30:30Z

⌛ Testing commit 4be4d4d with merge fc2c933...

…-pages, r=sgrif Exclude yanked crates on user/team pages Add a URL parameter to the `/api/v1/crates` endpoint that specifies whether or not the endpoint should return crates that have been fully yanked - i.e., where every version has been yanked. If not specified, this defaults to `true` because we have typically always returned all matching crates, whether or not their versions have been yanked. This makes it possible to display only non-yanked crates at various places in the UI without changing Rust code. Use this from the user and team pages to exclude yanked crates from them. Closes #958

bors · 2019-05-22T20:16:13Z

💔 Test failed - checks-travis

sgrif · 2019-05-22T20:20:09Z

Looks like beta broke the build, I'll fix it shortly

sgrif · 2019-06-13T20:46:28Z

@bors retry

bors · 2019-06-13T20:46:35Z

⌛ Testing commit 4be4d4d with merge dee30cb...

…-pages, r=sgrif Exclude yanked crates on user/team pages Add a URL parameter to the `/api/v1/crates` endpoint that specifies whether or not the endpoint should return crates that have been fully yanked - i.e., where every version has been yanked. If not specified, this defaults to `true` because we have typically always returned all matching crates, whether or not their versions have been yanked. This makes it possible to display only non-yanked crates at various places in the UI without changing Rust code. Use this from the user and team pages to exclude yanked crates from them. Closes #958

bors · 2019-06-13T21:09:15Z

💔 Test failed - checks-travis

bryanburgers · 2019-06-13T21:27:13Z

Boo! I cab look into the failure in a week or two.

sgrif · 2019-06-13T23:23:21Z

I think you might just need to rebase

Add a URL parameter to the `/api/v1/crates` endpoint that specifies whether or not the endpoint should return crates that have been fully yanked - i.e., where every version has been yanked. If not specified, this defaults to `true` because we have typically always returned all matching crates, whether or not their versions have been yanked. This makes it possible to display only non-yanked crates at various places in the UI, and is especially intended for the user and team pages to display only non-yanked crates.

Don't display crates that have been fully yanked - i.e., crates that where every version has been yanked - on user and team pages.

bryanburgers · 2019-06-20T21:19:51Z

@sgrif Rebased and fixed.

sgrif · 2019-06-20T21:33:57Z

@bors: r+

bors · 2019-06-20T21:33:58Z

📌 Commit 7b4e10e has been approved by sgrif

…-pages, r=sgrif Exclude yanked crates on user/team pages Add a URL parameter to the `/api/v1/crates` endpoint that specifies whether or not the endpoint should return crates that have been fully yanked - i.e., where every version has been yanked. If not specified, this defaults to `true` because we have typically always returned all matching crates, whether or not their versions have been yanked. This makes it possible to display only non-yanked crates at various places in the UI without changing Rust code. Use this from the user and team pages to exclude yanked crates from them. Closes #958

bors · 2019-06-20T21:34:05Z

⌛ Testing commit 7b4e10e with merge 60019d9...

bors · 2019-06-20T22:01:58Z

☀️ Test successful - checks-travis
Approved by: sgrif
Pushing 60019d9 to master...

sgrif reviewed Mar 1, 2019

View reviewed changes

bryanburgers force-pushed the exclude-yanked-crates-on-user-team-pages branch from 8da72e5 to 2ac226b Compare March 1, 2019 13:31

bryanburgers force-pushed the exclude-yanked-crates-on-user-team-pages branch from 2ac226b to 08ebb4b Compare March 2, 2019 13:38

bryanburgers force-pushed the exclude-yanked-crates-on-user-team-pages branch 2 times, most recently from 0e01147 to bcb4590 Compare April 5, 2019 19:34

carols10cents added the S-waiting-on-review label Apr 7, 2019

bryanburgers force-pushed the exclude-yanked-crates-on-user-team-pages branch from bcb4590 to 4be4d4d Compare May 13, 2019 13:29

bryanburgers force-pushed the exclude-yanked-crates-on-user-team-pages branch from 4be4d4d to be0a1e7 Compare June 20, 2019 20:34

bryanburgers added 2 commits June 20, 2019 15:59

Exclude yanked crates on user/team pages

7b4e10e

Don't display crates that have been fully yanked - i.e., crates that where every version has been yanked - on user and team pages.

bryanburgers force-pushed the exclude-yanked-crates-on-user-team-pages branch from be0a1e7 to 7b4e10e Compare June 20, 2019 20:59

bors merged commit 7b4e10e into rust-lang:master Jun 20, 2019

bryanburgers deleted the exclude-yanked-crates-on-user-team-pages branch June 21, 2019 11:38

carols10cents mentioned this pull request Apr 26, 2022

Crates where all versions are yanked do not show up in the list of the list of owned crates #4750

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Exclude yanked crates on user/team pages #1642

Exclude yanked crates on user/team pages #1642

bryanburgers commented Feb 28, 2019

sgrif Mar 1, 2019

bryanburgers Mar 1, 2019

bryanburgers commented Mar 1, 2019 •

edited

Loading

bors commented Mar 20, 2019

bors commented Apr 24, 2019

sgrif commented May 22, 2019

bryanburgers commented May 22, 2019

sgrif commented May 22, 2019

sgrif commented May 22, 2019

sgrif commented May 22, 2019

bors commented May 22, 2019

bors commented May 22, 2019

bors commented May 22, 2019

bors commented May 22, 2019

bors commented May 22, 2019

sgrif commented May 22, 2019

sgrif commented Jun 13, 2019

bors commented Jun 13, 2019

bors commented Jun 13, 2019

bryanburgers commented Jun 13, 2019

sgrif commented Jun 13, 2019

bryanburgers commented Jun 20, 2019

sgrif commented Jun 20, 2019

bors commented Jun 20, 2019

bors commented Jun 20, 2019

bors commented Jun 20, 2019

Exclude yanked crates on user/team pages #1642

Exclude yanked crates on user/team pages #1642

Conversation

bryanburgers commented Feb 28, 2019

sgrif Mar 1, 2019

Choose a reason for hiding this comment

bryanburgers Mar 1, 2019

Choose a reason for hiding this comment

bryanburgers commented Mar 1, 2019 • edited Loading

bors commented Mar 20, 2019

bors commented Apr 24, 2019

sgrif commented May 22, 2019

bryanburgers commented May 22, 2019

sgrif commented May 22, 2019

sgrif commented May 22, 2019

sgrif commented May 22, 2019

bors commented May 22, 2019

bors commented May 22, 2019

bors commented May 22, 2019

bors commented May 22, 2019

bors commented May 22, 2019

sgrif commented May 22, 2019

sgrif commented Jun 13, 2019

bors commented Jun 13, 2019

bors commented Jun 13, 2019

bryanburgers commented Jun 13, 2019

sgrif commented Jun 13, 2019

bryanburgers commented Jun 20, 2019

sgrif commented Jun 20, 2019

bors commented Jun 20, 2019

bors commented Jun 20, 2019

bors commented Jun 20, 2019

bryanburgers commented Mar 1, 2019 •

edited

Loading