Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[settings-view] Use new owner endpoint instead of users #857

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

confused-Techie
Copy link
Member

This PR builds off of @savetheclocktower's hard work on pulsar-edit/package-backend#215 which allows us to filter owners of package's via query parameter.

So this PR ensures when we link to a code owner, instead of going to their user page, we instead go to the packages page with the proper query parameter set.

@confused-Techie confused-Techie changed the title [settings-view] Use new owner query parameter instead of old users endpoint [settings-view] Use new owner endpoint instead of users Jan 8, 2024
@DeeDeeG
Copy link
Member

DeeDeeG commented Jan 11, 2024

If I visit https://web.pulsar-edit.dev/owners/DeeDeeG, I get

This Page encountered an Error!
If you think you're seeing this page in error, please report an issue on [GitHub](https://github.com/pulsar-edit/package-frontend/issues) or [Discord](https://discord.gg/7aEbB9dGRT).

        The page '/owners/DeeDeeG' cannot be found.

Is that expected? Does the DB need to be seeded with owner metadata on packages or something? I also tried it with savetheclocktower's and spiker985's usernames, since I figured they would have published packages since pulsar-edit/package-backend#215 landed, and the metadata would presumably be populated at the DB for at least those pacakges?

@savetheclocktower
Copy link
Contributor

savetheclocktower commented Jan 11, 2024

To clarify: the frontend part isn't implemented yet (though the owner URL param does work to filter by owner on package search pages). I plan to get to it soon.

I think when this link was put in, the expectation was that the /users/:user endpoint would eventually exist, but it's just been linking to nothing for ages. This improves the situation because at least we have a theoretical plan in place to make this endpoint. :-)

Copy link
Member

@DeeDeeG DeeDeeG left a comment

Choose a reason for hiding this comment

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

Very simple change, reportedly in anticipation of some package registry frontend work to make this endpoint (these URLs being linked to here) actually active.

No reason not to do this, then, I suppose. LGTM!

@confused-Techie
Copy link
Member Author

Yeah thanks for taking a look at this @DeeDeeG, and clarifying the situation @savetheclocktower

Suppose I probably should've made this a draft to indicate more work was needed. But am planning to wait until the frontend work becomes active (Which itself also relies on one more backend change) prior to merging. But thanks for the approval!

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