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

Content directory schema: Entity entity defination #1400

Closed
wants to merge 14 commits into from

Conversation

metmirr
Copy link
Contributor

@metmirr metmirr commented Sep 17, 2020

This PR introduces entity definition for Entity type to the query node schema and addresses this issue #1341.

The PR is built on #1399

@metmirr metmirr self-assigned this Sep 17, 2020
@metmirr metmirr requested review from iorveth and bedeho September 18, 2020 15:08
controller: EntityController!

"Forbid groups to mutate any property value"
isFrozen: Boolean!
Copy link
Contributor

Choose a reason for hiding this comment

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

"Current controller, which is initially set based on who created entity"
	controller: EntityController!
	"Forbid groups to mutate any property value"
	isFrozen: Boolean!
"Prevent from being referenced by any entity (including self-references)"
	isReferenceable: Boolean!

Can we move EntityPermissions related fields to separate type, please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we have EntityPermissions type there is no use except in Entity so I think it's good to keep the fields here.

Copy link
Contributor

@iorveth iorveth Sep 21, 2020

Choose a reason for hiding this comment

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

I don`t think someone will query only entity permissions, though it seems to be easier to select all permissions related fields, when he/she need them.

query {
    entity {
      entityId
      entityPermissions
  }
}

instead of

query {
    entity {
      entityId,
     controller: EntityController!
	"Forbid groups to mutate any property value"
	isFrozen: Boolean!
"Prevent from being referenced by any entity (including self-references)"
	isReferenceable: Boolean!
  }
}

Will it complicate things a lot?

"""
Fields encode a flattened representation of vector properties, only one will be non-null
"""
type VectorValue @variant {
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't sure, if i fully understand how @variant directive works.
Though i concerned about the nonce: BigInt! field here.
We should have this field for each of VectorValue variants.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we will have nonce (non-null) field for each variants. You can find more on @variant directive there https://dzhelezov.gitbook.io/hydra-docs/docs/schema-spec/variant-types

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, great

@iorveth iorveth self-requested a review September 22, 2020 08:22
Copy link
Contributor

@iorveth iorveth left a comment

Choose a reason for hiding this comment

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

LGTM!

@metmirr
Copy link
Contributor Author

metmirr commented Sep 24, 2020

Superseded by #1458

@metmirr metmirr closed this Sep 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants