Skip to content
This repository has been archived by the owner on Aug 1, 2022. It is now read-only.

feat(proxy): extend project with org/user relation #251

Merged
merged 3 commits into from Mar 25, 2020

Conversation

xla
Copy link
Contributor

@xla xla commented Mar 25, 2020

Closes #245

@xla xla added proxy feature Something that doesn't exist yet labels Mar 25, 2020
@xla xla self-assigned this Mar 25, 2020
@xla xla added this to In progress in weekly via automation Mar 25, 2020
MeBrei
MeBrei previously requested changes Mar 25, 2020
Copy link
Contributor

@MeBrei MeBrei left a comment

Choose a reason for hiding this comment

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

LGTM! Just a small addition

proxy/tests/graphql_mutation.rs Show resolved Hide resolved
weekly automation moved this from In progress to In review Mar 25, 2020
match registration {
project::Registration::Org(org_id) => {
Some(ProjectRegistration::Org(OrgRegistration {
org_id: juniper::ID::new(org_id.to_string()),
Copy link
Member

Choose a reason for hiding this comment

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

Could we return the actual org/user instead of just the id and maybe mock out some sample data?

So far it looks like we'll need:

  • user handle / org name
  • avatarUrl
  • avatarFallback

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To mock this meaningfully we need orgs first. For the rest we have to decide what behaviour we want as:

  • neither user nor orgs have avatar urls
  • only orgs have a fallback avatar
  • only users maybe have an coco identity attested to them
    • which only maybe is available at the time of query

What we can do for now is to show the foo / bar info with a returned avatar from the avatar query. How we control that a project appears as registered is open for discussion, it could be a query parameter that we allow for now.

Copy link
Member

Choose a reason for hiding this comment

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

neither user nor orgs have avatar urls

Good point, forgot about that.

only orgs have a fallback avatar

Why wouldn't a user have a fallback avatar?

What we can do for now is to show the foo / bar info with a returned avatar from the avatar query.

Deal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why wouldn't a user have a fallback avatar?

AFAIK only identities have fallback avatar, the handle more specifically. @cloudhead and @MeBrei can give more context.

Copy link
Member

@rudolfs rudolfs Mar 26, 2020

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

I guess this will be answered in: #230.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rudolfs We have user emojis for the identity handles.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Expanded on in #230 (comment)

@xla xla requested a review from MeBrei March 25, 2020 15:20
@xla xla dismissed MeBrei’s stale review March 25, 2020 15:20

Feedback addressed.

weekly automation moved this from In review to Approved Mar 25, 2020
@xla xla merged commit 7c2a424 into master Mar 25, 2020
weekly automation moved this from Approved to Done Mar 25, 2020
@xla xla deleted the xla/245-extend-registered branch March 25, 2020 23:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature Something that doesn't exist yet
Projects
No open projects
weekly
  
Done
Development

Successfully merging this pull request may close these issues.

Extend Project query with User/Org relation
4 participants