Hypertopic / Porphyry

Corpus analyses confrontation
https://hypertopic.org/porphyry
GNU Affero General Public License v3.0
21 stars 165 forks source link

Authorize a contributor to edit a viewpoint #183

Closed benel closed 2 years ago

benel commented 5 years ago

Rules:

  1. Viewpoints with a list of contributors can be edited only by them (but existing viewpoints, without a list or with an empty list, are still writable by any authenticated user).

  2. On creation, a viewpoint has its author as its only contributor.

  3. Any contributor of a viewpoint can authorize another user to contribute.

Note: Porphyry feature is only the user interface part of a "real" authorization performed by Argos (see https://github.com/Hypertopic/Argos/commit/97deb65867ac7fac05d05d7ce51ad4c81e7ff7bb).


Phase 1

ThomasLORIOT commented 4 years ago

Young Artist : Permettrait à des psychologues de partager rapidement des dessins selon leur terme de recherche et de contribuer à ce point de vue entre eux.

melaniehoneychurch commented 4 years ago

Cette fonctionnalité est utile pour le projet car il s'agit d'un travail collaboratif il parait donc utile que plusieurs collaborateurs puissent modifier un même point de vue.

benel commented 4 years ago

@melaniehoneychurch Tout à fait. Cependant pour être tout à fait précis, pour l'instant tout est modifiable par tout le monde (comme un wiki). Cette nouvelle fonctionnalité permettrait donc plutôt d'autoriser uniquement quelques personnes à le faire (par défaut l'auteur, comme sur Google Docs).

MaximeD89 commented 4 years ago

@benel Je pense que ce point va de pair avec le ticket :

Register as a contributor #184

En réalisant ces deux points, il nous sera donc possible de n'autoriser que l'auteur par défaut à modifier un point de vue. (Identification par compte d'utilisateur).

Puis si ce dernier le souhaite il pourra autoriser des collaborateurs qui seront qualifiés de "fiables" à modifier son point de vue.

benel commented 4 years ago

@MaximeD89 Oui c'est ça.

sarah-ngn commented 3 years ago

Graines d'artistes : Pourrait permettre à un groupe de personnes de travailler sur le même point de vue. Ce point de vu sera créé/modifié par le groupe et pour le groupe.

benel commented 3 years ago

@sarah-ngn @Hypertopic/graines-d-artistes-1 Oui, ça a du sens.

On est d'accord que pour l'instant tout le monde peut tout éditer ? Là ça permettrait de décider qui peut le faire.

antoinethz commented 3 years ago

Etude à l'étranger : Permettre à chaque utilisateur de créer et modifier ses propre viewpoint pour son utilisation personnel.

Samnoel95 commented 3 years ago

Compétence EUt+ :

Cette feature est intéressante pour notre projet. Le catalogue est collaboratif, il est donc important d'autoriser les utilisateurs à créer des points de vues. De plus le contributeur pourra choisir de partager ou non les droits de modification de son point de vue.

Xelocks commented 3 years ago

Groupe Etudes à l'étranger : Avec @ThomasRitaine, nous avons commencé à travailler sur une stratégie d'implémentation. Voici le résultat.

Quelle partie du code sera impactée (classes, méthodes) ?

Y aura-t-il des difficultés d'un point de vue algorithmique ? Avez-vous des pistes pour les résoudre ?

Un système de viewpoint existe déjà, mais ce ne sont pas des entités qui peuvent posséder des attributs tels que le créateur, la date de création, la visibilité (privé, publique…) Ainsi, il va falloir modifier la façon dont sont stockés les points de vue. La liste des attributs que nous voulons ajouter aux points de vue est assez courante, et ne devrait pas poser de problème d’un point de vue algorithmique.

Les données dont vous avez besoin sont-elles déjà chargées par la page ? Si oui, quelle est la structure des données et comment allez-vous récupérer précisément celles qui vous intéressent ? Si non, quelle sera la requête la plus efficace et comment allez-vous récupérer dans la structure de données précisément celles qui vous intéressent ?

Non, les données dont nous avons besoin ne sont pas directement chargées sur la page. Il va falloir modifier le backend, et créer le type de réponse que nous souhaitons. Par exemple, pour récupérer la liste des viewpoints, nous pourrons avoir une réponse json avec la forme suivante :

{
"pointOfViewList" : 
  [
    {
      "id" : cef52507-de32-444a-97fc-2de092ed65d3,
      "name" : Mon point de vue,
      "group" : Mes points de vue,
      "visibility" : public,
      "author" : ritainet
    },
    {
      "id" : 123e4567-e89b-12d3-a456-426614174000,
      "name" : Mon point de vue,
      "group" : Asie,
      "visibility" : public,
      "author" : beurtonj
    }
  ]
}

Existe-t-il des bibliothèques qui pourraient simplifier l'implémentation ?

L’ajout de champs pour les viewpoints ne nécessite pas de librairie particulière. Il y aura un travail sur l’architecture de la base de données pour ajouter aux points de vue un nom, un auteur, un groupe et une visibilité.

Comme dans notre séance sur le développement, vous pouvez tester des petits bouts de code indépendants et les associer à votre stratégie (avec éventuellement des exemples de données, voire même des temps d'exécution).

Pas de code testé.

Doit-on intégrer des services extérieurs (ex : cartographie) ? De quelles fonctionnalités en particulier aura-t-on besoin ? Est-ce que le service choisi les propose ou doit-on passer chez un concurrent ? Comment s'utilisent ces fonctionnalités ? avec quels paramètres ?

Aucun service extérieur ne sera nécessaire.

Des étapes préalables de développement, testables, sont-elles envisageables afin de diminuer la complexité de la livraison ? Refactoring (livrable à part entière, isofonctionnel) ? Code développable et testable de manière externe au logiciel ? Étape du scénario (non livrable en tant que tel mais dont l’effet peut être testé) ?

Rien à tester.

benel commented 3 years ago

@Antoine-thz @TheoHoenen

Merci pour la première version de vos scénarios.

Au-delà des problèmes de forme que j'ai indiqué ligne à ligne, je crois qu'il y a un problème plus profond de compréhension de ce que veut dire "Authorize a contributor to edit a viewpoint". Je vous rappelle que le sujet implicite du verbe d'une fonctionnalité est toujours un utilisateur (ce n'est pas le système qui est "sujet"). Du coup, la phrase complète serait : A autorise B à éditer le point de vue X.

Dit autrement vos scénarios (les différents cas) doivent être rédigés autour de déclencheurs de ce type :

Quand "alice" autorise "bob" à éditer le point de vue "Équivalences UTT"
michaelmtre commented 3 years ago

@benel

Avec @gb99 nous avons fait à nouveau les ébauches de nos maquettes suite aux commentaire sur les précédentes. N'hésitez pas à nous faire des remarques dans les commentaires 👍


Tout d'abord, l'utilisateur décide de sélectionner un point de vue (ici "Informatique") et clique sur le bouton de modification, qui existe déjà.

Authorize_1

On se retrouve sur la page d'édition du point de vue "Informatique" dans laquelle une nouvelle section "Contributeurs" a été créée. Cette section contient :

Authorize_2 Authorize_3
paasshme commented 3 years ago

@benel voici la réponses aux différentes questions de la stratégie d'implémentation que nous avons réalisé avec @Anas9820 (début de la discussion trouvable ici)

Pour la suite de la stratégie d'implémentation nous avons répondu aux trois point mentionnés dans le message précédent:

1) de récupérer la liste des contributeurs autorisés pour un point de vue donné, 2) de récupérer (si c'est possible techniquement, conforme aux règles de sécurité et déjà implémenté) la liste des utilisateurs disponibles (ou à défaut déjà autorisés pour d'autres points de vue), 3) de modifier la liste des contributeurs d'un point de vue donné.

Nous avions convenu lors de la dernière réunion technique qui a suivi la réunion de vendredi dernier que le point n°2 ne serait pas abordé dans le cadre de ce ticket. L'utilisateur devrait donc rentrer le nom exact du contributeur qu'il veut ajouter. Ce choix est fait car la récupération de tous les utilisateur s'avère complexe techniquement. Ce choix est fait également car l'utilisateur serait amené à discuter au préalable avec un utilisateur voulant être un contributeur. Durant cette discussion le futur contributeur aurait simplement à donner son nom afin d'être ajouté ultérieurement.


Nous avons réalisé un module javascript afin d'explorer les possibilités pour les points 1) et 3). Il est trouvable en entier ici: gist entier Ce fichier contient des fonctions utilisés pour tester les fonctionnalités, celles-ci seront sujet à modification car elles ont été écrite dans une perspective d'exploration de l'API et de tests facilités.

Dans ce fichier on peut trouver notamment une fonction qui permet de récupérer la liste des contributeurs autorisés pour un point de vue donné: fonction

// Get contributors of a viewpoint (default id is id of a viewpoint with the test data)
const getContribs = async (id='a76306e4f17ed4f79e7e481eb9a1bd06') => {
    let viewpoint = await db.auth('alice', 'whiterabbit').get({_id:id}).catch(_error);     
    return viewpoint.contributors
}

Cette fonction répond donc au point 1) et permet de récupérer un tableau de nom de personne (ou undefined dans le cas ou aucun contributeur n'est présent)


Pour le point 3) et donc la modification des contributeurs, nous avons réalisé trois fonctions:

// Add contributors to a viewpoint    
const addContributors = async(viewpointId, newContributors) => {   
   // On récupère les contributeurs actuels
    const currentContributors = await getContribs(viewpointId)     
    const set = new Set(currentContributors)
    // Ajout des nouveaux contributeurs
    newContributors.forEach(c => set.add(c))
   // Récupération du viewpoint
    let viewpoint = await  db.auth('alice', 'whiterabbit').get({_id:viewpointId}).catch(_error);
   // Ajout des contributors au viewpoint
    viewpoint.contributors = [...set]    
   // Mise à jour des données
    await db.post(viewpoint).catch(_error)     
 // Affichage des contributeurs mis à jour
    console.log(await getContribs(viewpointId))
}

  // Remove contributors from a viewpoint
  const removeContributors = async (viewpointId, contributorsToRemove)  => {
    const currentContributors = await getContribs(viewpointId)
    const set = new Set(currentContributors)
    contributorsToRemove.forEach(c => set.delete(c))

  // Récupération du viewpoint
    let viewpoint = await  db.auth('alice', 'whiterabbit').get({_id:viewpointId}).catch(_error);
// Modification des contributeurs (on remplace l'ancien tableau par le tableau modifié)
    viewpoint.contributors = [...set]
    // Passage en mode wiki du viewpoint dans le cas ou tous les contributeurs sont supprimés
    if (viewpoint.contributors.length === 0)
      viewpoint.contributors = undefined
 // Mise à jour des données sur le serveur
    await db.post(viewpoint).catch(_error)
// Affichage des contributeurs
    console.log(await getContribs(viewpointId))
  }
// Set contributors to undefined (wiki mode)
const resetContributors = async (viewpointId) => {
    let viewpoint = await  db.auth('alice', 'whiterabbit').get({_id:viewpointId}).catch(_error);
   // On supprime l'attribut contributors du viewpoint
    delete viewpoint.contributors
    const res = await db.post(viewpoint).catch(_error)
    console.log(`Contributors are now: ${res.contributors ? res.contributors : 'everyone (wiki mode)'}`)
}

Lors de la création d'un viewpoint, l'idée serait d'utiliser la première fonction avec le nom de celui-ci afin que celui-ci soit le seul contributeur sur son viewpoint.

Vous pouvez tester toutes ces fonctions en lançant simplement argos & la couchdb. N'hésitez pas à nous faire des remarques dans les commentaires :+1:

benel commented 3 years ago

@michaelmtre @gb99

Merci pour ces nouvelles maquettes.

119558495-be98c780-bda1-11eb-8ad5-9be67db91a0c

OK. Oui, c'est pas mal du tout. Les interactions sont tout à fait raisonnables et décrites de manière suffisamment précise pour une implémentation et des tests.

Peut-être que ceux qui vont implémenter voudront négocier quelques détails de mise en page (les marges notamment) qui compliquent un peu les choses sans que l'on voit très clairement ce que cela apporte). Mais c'est mineur et la négociation pourra avoir lieu à ce moment-là.

benel commented 3 years ago

@michaelmtre @gb99

Peut-être juste revoir le placement de la zone des contributeurs. En effet, comme il s'agit d'une annexe à la zone principale, vous pourriez réfléchir à la mettre à la droite ou à la gauche de la zone principale... D'autant plus que quand le point de vue est rempli, la page est très chargée verticalement et très peu horizontalement. Ex :

Capture d’écran 2021-05-26 à 10 31 17
benel commented 3 years ago

@JacquesMironneau @Anas9820

de récupérer (si c'est possible techniquement, conforme aux règles de sécurité et déjà implémenté) la liste des utilisateurs disponibles (ou à défaut déjà autorisés pour d'autres points de vue),

Nous avions convenu lors de la dernière réunion technique qui a suivi la réunion de vendredi dernier que le point n°2 ne serait pas abordé dans le cadre de ce ticket. L'utilisateur devrait donc rentrer le nom exact du contributeur qu'il veut ajouter. Ce choix est fait car la récupération de tous les utilisateur s'avère complexe techniquement. Ce choix est fait également car l'utilisateur serait amené à discuter au préalable avec un utilisateur voulant être un contributeur. Durant cette discussion le futur contributeur aurait simplement à donner son nom afin d'être ajouté ultérieurement.

Tout à fait. Il était important de garder une trace de cette décision, comme vous le faites là, car c'est un des points majeurs de votre stratégie d'implémentation.

benel commented 3 years ago

@JacquesMironneau @Anas9820

C'est très bien d'avoir fait tous ces tests avec des extraits de code !

https://gist.github.com/JacquesMironneau/8c35c2dbf90e5ab3d96e39ba59971f0a#file-contributor-js-L12-L16

Notez que l'authentification HTTP :

Par ailleurs, pour ce point précis, normalement ce sont des données que vous avez déjà récupérées. Vous pouvez observer dans les outils de développement Web de votre navigateur le résultat des requêtes lancées. Par contre il faudrait identifier l'endroit où est fait la requête dans le code pour ensuite planifier la transmission de ces données jusqu'à votre composant.

paasshme commented 3 years ago

Par ailleurs, pour ce point précis, normalement ce sont des données que vous avez déjà récupérées

De quelles données parlez-vous ? Celle d'authentification ?

Par contre il faudrait identifier l'endroit où est fait la requête dans le code pour ensuite planifier la transmission de ces données jusqu'à votre composant.

Vous parlez de la requête d'authentification ? Ou voulez vous dire que la requête récupérant les contributeurs existe déjà dans le code de porphyry ?

benel commented 3 years ago

@JacquesMironneau @Anas9820

https://gist.github.com/JacquesMironneau/8c35c2dbf90e5ab3d96e39ba59971f0a#file-contributor-js-L26-L36

C'est effectivement ce premier bout de code qui correspond à ce que l'on veut faire. Les deux autres, bien qu'intéressants correspondent à de futures fonctionnalités (si certains de votre équipe n'avaient pas encore créé de tickets, c'est l'occasion !).

Ça semble bien faire ce que c'est censé faire... Bravo !

Mais pourriez-vous essayer maintenant de l'écrire dans le "style fonctionnel" de la bibliothèque, c'est à dire, sous la forme d'une séquence d'étapes ou chacune attend le résultat de la précédente avec then pour devenir les paramètres d'entrée de la suivante ?

Petits conseils :

Si vous bloquez à une des étapes. Indiquez moi l'étape précédente qui fonctionne et la suivante qui ne fonctionne pas et je tenterai de vous "débloquer".

benel commented 3 years ago

@JacquesMironneau @Anas9820

Par ailleurs, pour ce point précis, normalement ce sont des données que vous avez déjà récupérées

De quelles données parlez-vous ? Celle d'authentification ?

Non, non, je parle bien des contributeurs autorisés du point de vue en édition.

Capture d’écran 2021-05-26 à 11 28 01

Mais on est d'accord que vous avez quand même besoin de cette étape de lecture avant de faire la modification. Sinon vous risqueriez d'avoir des conflits de mise à jour (s'il y a eu d'autres modifications en parallèle).

Je vous invite d'ailleurs pour l'instant, comme je le disais plus haut, à présenter votre "authentification / lecture / modification en mémoire / modification sur le serveur" en un seul fil d'exécution (un peu comme dans l'exemple "Update an object" de la documentation de la bibliothèque).

paasshme commented 3 years ago

D'accord merci je vois ce que voulez dire pour la récupération de données.

Oui je vais réécrire le tout avec des then.

paasshme commented 3 years ago

Mais on est d'accord que vous avez quand même besoin de cette étape de lecture avant de faire la modification. Sinon vous risqueriez d'avoir des conflits de mise à jour (s'il y a eu d'autres modifications en parallèle).

La récupération des données inclurait donc uniquement l'id du viewpoint puis dans le composant nous ferions la requête récupérant les contributeurs en fonction de cet id?

paasshme commented 3 years ago

@benel voici la version écrite sans async/await

    db.auth('alice', 'whiterabbit')
    .get({_id:'a76306e4f17ed4f79e7e481eb9a1bd06'})
    .then(x => {
      const set = new Set(x.contributors)
      const newContributors = ['alice','anas','benel']
      newContributors.forEach(c => set.add(c))
      x.contributors = [...set]
      return x;
    })
    .then(db.post)
    .then(x => _log(x.contributors))
    .catch(_error); 

Est-ce que cela vous convient ?

benel commented 3 years ago

@JacquesMironneau

La récupération des données inclurait donc uniquement l'id du viewpoint puis dans le composant nous ferions la requête récupérant les contributeurs en fonction de cet id?

Je ne sais pas trop si je comprends votre question. Je reformule :

Est-ce que cela vous convient ?

Oui c'est beaucoup mieux !!!

Il y a juste le forEach qui me chagrine un peu. Mais vous pouvez facilement le supprimer en ajoutant non pas plusieurs contributeurs à la fois mais un seul. Oui, je sais bien que ce serait intéressant dans l'absolu. Mais vous préparez l'implémentation d'une fonctionnalité qui n'en mentionne qu'un. Là encore : nouveau ticket !

paasshme commented 3 years ago

Je ne sais pas trop si je comprends votre question. Je reformule :

  • Pour l'affichage (render), la requête a déjà été faite. Il faut juste que les contributeurs soient transmis dans l'état de votre composant.
  • Pour la modification, on refait effectivement une lecture (pour être sûr d'avoir la toute dernière version) et on enchaîne tout de suite avec la modification ciblée et l'écriture.

C'est bien ce que j'avais compris, merci.

Voici la version avec un seul contributeur:

    db.auth('alice', 'whiterabbit')
    .get({_id:'a76306e4f17ed4f79e7e481eb9a1bd06'})
    .then(x => {
      x.contributors = [...new Set(x.contributors).add('toto')]
      return x;
    })
    .then(db.post)
    .then(x => _log(x.contributors))
    .catch(_error); 
benel commented 3 years ago

@JacquesMironneau

$ node
Welcome to Node.js v14.17.0.
Type ".help" for more information.
> let x = {}
undefined
> [...new Set(x.contributors).add('toto')]
[ 'toto' ]
>

Et vous gérez le cas où le point de vue était en mode wiki. Bravo !

paasshme commented 3 years ago

@benel Après avoir creuser dans le code de porphyry (plus précisémment dans le Component Outliner) je pense que nous pourrions modifier la fonction _fetchData.

  async _fetchData() {
    if (!this.changing) {
      new Hypertopic((await conf).services)
        .get({ _id: this.props.match.params.id })
        .then(x => {
          // On donc remplacer cette ligne  
          this.setState({ topics: x.topics, title: x.viewpoint_name });
         // par this.setState({ topics: x.topics, title: x.viewpoint_name, contributors: x.contributors });
          this.topicTree = new TopicTree(x.topics);
        });
    } else {
      return true;
    }
  }

De cette manière on pourrait ajouter notre composant dans la méthode render de l'outliner et passer par exemple en props les contributors, et/ou l'id du viewpoint.

benel commented 3 years ago

@Hypertopic/etudes-a-l-etranger-1 @Hypertopic/competences-eut

Vous aviez remarqué que vous étiez en concurrence sur les mêmes livrables ?

En tout cas, pour ce qui concerne les scénarios, ce sont ceux de @candalc et @Samnoel95 qui sont les plus aboutis. Ce sont donc ceux-là que je vais intégrer.

Pour les autres livrables, essayez d'être "plus coopératifs dans la concurrence". Vous pouvez par exemple :

GuenaultAlexandre commented 3 years ago

Voici nos maquette (posté avec du retard désolé) Fait avec @Nicflx

Disclaimer : notre vision de ce ticket dans notre équipe était majoritairement tourné autour de la création d'un point de vue personnel d'où, cf le post ci dessous de Antoine

Etude à l'étranger : Permettre à chaque utilisateur de créer et modifier ses propre viewpoint pour son utilisation personnel.

Voici ce que nous avions imaginé :

image

Ici on voit donc que l'on peut ajouter un point de vue avec un nom, une description et on peut ajouter plusieurs items via des champs éditables. Il suffit de valider l'insertion (soit par l'appuie de la touche entré ou clic en dehors du champ cliquable, dans la pire des cas on peut ajouter un bouton de validation si pas assez user-friendly) Après cela l'item est bien inséré et on peut continuer à créer de nouveaux items.

image

benel commented 3 years ago

@GuenaultAlexandre @Nicflx

notre vision de ce ticket dans notre équipe était majoritairement tourné autour de la création d'un point de vue personnel

Ce n'est pas grave de se tromper (ça peut arriver à tout le monde, à condition de comprendre la raison de l'erreur pour ne plus la refaire. Comment interprétez-vous votre mauvaise compréhension du titre de ce ticket ("authorize" ≠ "create") ?

GuenaultAlexandre commented 3 years ago

@benel Je ne me souviens pas exactement comment on en est arrivé à ce problème de compréhension mais si je devais poser une hypothèse sur cela, Il me semble que a un moment donnée on voulais donner la possibilité à certains utilisateurs de modifier les points de vue au cas où certains éléments changent dans le temps et ainsi réduire la charge de travail à ceux qui ont crée les points de vue. Sauf qu'au fur et à mesure des semaines, les réflexions ont évolué, ainsi que notre compréhension des besoins, mais pas les tickets présents sur notre kanban, on a certainement donc confondu le besoin cité précédemment dans ce ticket avec celui qu'on avait à l'origine. Notre erreur fut alors de laisser ce ticket.

Je laisse quelqu'un me corriger si je me trompe mais il me semble bien que c'est ce qu'il s'est passé.

benel commented 3 years ago

@GuenaultAlexandre @Nicflx

Peut-être aussi qu'à un moment vous avez oublié que le verbe d'une fonctionnalité est une action de l'utilisateur et non du système (c'est l'utilisateur qui autorise l'édition à un autre utilisateur à éditer et non juste le système qui permet l'édition) ?

Du coup, est-ce que ce ticket garde son intéret pour votre domaine d'application (@Hypertopic/etudes-a-l-etranger-1) ? Et si non, souhaitez-vous planifier dans votre kanban un ticket qui correspondrait plus à ce que vous me disiez à propos des points de vue personnels (la fonctionnalité #50 par exemple).

benel commented 3 years ago

Félicitations @JacquesMironneau et @Anas9820 🏅 Votre implémentation de la fonctionnalité est désormais intégrée à la branche principale 🎉

benel commented 3 years ago

La fonctionnalité est en production 🎉