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

Show aggregated independent perspectives on the wiki perspective list #216

Open
7 of 10 tasks
pepoospina opened this issue Sep 24, 2020 · 4 comments
Open
7 of 10 tasks
Assignees

Comments

@pepoospina
Copy link
Contributor

pepoospina commented Sep 24, 2020

The idea is to see something like this on the wiki home page.
https://www.figma.com/file/4iz8qX6voSNg0EvEMQCyMj/Multi-Perspective-Wiki?node-id=33%3A7

(The proposals will still be shown on a different section for the moment)

This will require to

  • change the server PUT /persp endpoint that currently filters perspectives, to receive another query parameter, next to the context one which is includeEcosystem.
  • update the backend to return all the independent perspectives of the ecosystem of a perspective and return their ids in an array.
  • Filter the independent perspectives to those for which the current logged user canRead: this is, they are publicRead, or the user is in canRead, canEdit or canAdmin "arrays".

Now that we have all the other perspectives under a perspective ecosystem in the frontend, this will get listed inside the evees-perspective-list component.

  • change that component to show, on each renderPerspectiveRow(), a new independent component called <evees-perspectives-row>, which receives the uref of the perspective and will render its details. Use the evees-proposals-list + evees-proposal-row pair of components as an example.

In the evees-perspectives-row :

  • Add another attribute context that will keep the context of the wiki.
  • fetch the details of the perspective under a load() method called on firstUpdated()
  • if the context of a perspective is not the context of the wiki, then show of ${elementTitle} after the <evee-author>, where element title is derived from the uref and the title pattern.
  • Fire at will on the styling.

Finally,

  • When a user clicks on a perspective of an element of the ecosystem, show that element as the current page (temporary solution).
  • this will require you to check the way the selected page is shown and extend that logic to be able to show also an arbitrary uref (received from the checkout-perspective method), even if it is not a real page of the wiki.
@sotous sotous self-assigned this Sep 25, 2020
@pepoospina
Copy link
Contributor Author

There is an issue with the ambiguity of deriving if a user canRead when the user is only under the canWrite or canAdmin vertices.

There seems to be a few options in terms of refactoring the DB to better handle this case:

  1. Filtering manually after the query returns all objects.
  2. Add a top level @filter with the logic @filter gt(len(canAdmin), 0) OR gt(len(canWrite), 0) OR gt(len(canRead), 0)
  3. Refactor permissions approach to add the did of an Admin (or Editor) to the canRead and canWrite arrays.
  4. Just a note. Would flatten the permissions to the perspective level help to simplify the queries? This would be second refactor.

@sotous
Copy link
Member

sotous commented Sep 28, 2020

The idea is to avoid any loop inside the server logic, however, I got to improve efficiency trying to make solution number 2, which might not be possible: uprtcl/js-uprtcl-server@develop-merge...agreggated-ind-persp.

Here is why:

  • Let's suppose we have the following query:
{
  persp(func: eq(context, "perspective.A.context")) {
    xid
    accessConfig {
      permissions 
      @filter(gt(count(canAdmin), 0) OR gt(count(canRead), 0) OR gt(count(canWrite), 0))
     {
        publicRead
        publicWrite
        canAdmin @filter(eq(did, "THE_DID")) {
          did
        }
        canRead @filter(eq(did, "THE_DID")) {
          did
        }
        canWriten @filter(eq(did, "THE_DID")) {
          did
        }
      }
    }    
  }
}

Dgraph after knowing that there is one administrator, it will still check filters for canRead and canWrite, despite finding out that those arrays are empty because it checks at a previous level, ignoring what is deeper in the next level. The only way permissions would be ignored during a query, is that all three conditions were not given, this way dgraph won't return the permissions predicate, still, this is a concept type situation, since all canAdmin predicates must at least have one node to point.

There is an issue with the ambiguity of deriving if a user canRead when the user is only under the canWrite or canAdmin vertices.

There seems to be a few options in terms of refactoring the DB to better handle this case:

  1. Filtering manually after the query returns all objects.
  2. Add a top level @filter with the logic @filter gt(len(canAdmin), 0) OR gt(len(canWrite), 0) OR gt(len(canRead), 0)
  3. Refactor permissions approach to add the did of an Admin (or Editor) to the canRead and canWrite arrays.
  4. Just a note. Would flatten the permissions to the perspective level help to simplify the queries? This would be second refactor.

@sotous
Copy link
Member

sotous commented Oct 3, 2020

In this PR, we make tests for independent perspectives, and, add the permissions check in the dgraph query. The PR here, uprtcl/js-uprtcl-server#33, however, we still have to finish collecting real facts to determine if the refactor when setting up an administrator is already complete, I would suggest this as the following step to follow before continuing with the issue.

@pepoospina
Copy link
Contributor Author

pepoospina commented Nov 16, 2020

This will require to

  • add a new endpoint to the server PUT /persp/:perspId/others that will return the other perspectives of a perspective. Include a query parameter includeEcosystem to include the other perspectives of the ecosystem or not.

  • change the getContextPerspectives(context: string) method of the EveesRemote and replace it with getOtherPerspectives(perspectiveId: string). (updated with stubs all the other providers so that they compile. @pepoospina will update those.)

  • change the EveesPerspectivesList component to use the recent method of all registered remotes instead of the current query to get the other perspectives. Aggregate the results from all the remotes similar to how it is done on the resolver.

Now that we have all the other perspectives under a perspective ecosystem in the frontend, this will get listed inside the evees-perspective-list component.

  • change that component to show, on each renderPerspectiveRow(), a new independent component called <evees-perspectives-row>, which receives the uref of the perspective and will render its details. Use the evees-proposals-list + evees-proposal-row pair of components as an example.

In the evees-perspectives-row :

  • Add another attribute context that will keep the context of the wiki.
  • fetch the details of the perspective under a load() method called on firstUpdated()
  • if the context of a perspective is not the context of the wiki, then show of ${elementTitle} after the <evee-author>, where element title is derived from the uref and the title pattern.
  • Fire at will on the styling.

Finally (TBC),

  • When a user clicks on a perspective of an element of the ecosystem, show that element as the current page (temporary solution).
  • this will require you to check the way the selected page is shown and extend that logic to be able to show also an arbitrary uref (received from the checkout-perspective method), even if it is not a real page of the wiki.

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

No branches or pull requests

2 participants