assemblee-virtuelle / archipelago

Fostering interconnections between communities by creating synergies between their platforms
Apache License 2.0
14 stars 6 forks source link

[Major] (v2.0.0) Improve configuration for v2 #177

Closed mguihal closed 2 months ago

mguihal commented 3 months ago

Hello, je propose cette dernière PR pour la v2.0.0 d'Archipelago.

Pourquoi cette PR ?

Dans le futur, il est prévu que les fichiers d'Archipelago passe de javascript à Typescript, (et donc changent d'extension), mais les instances d'Archipelago actuellement peuvent surcharger les fichiers pour les customiser (comme c'est décrit dans le dossier /deploy avec le dossier app dédié. Hors cette surcharge rend impossible le changement ultérieur de nom des fichiers ainsi modifiés. Je profite donc de cette version majeure pour changer la façon dont on appréhende la surcharge des fichiers afin qu'on puisse faire les prochaines évolutions sans breaking changes.

L'objectif de cette PR est de centraliser toutes ces surcharges via le fichier src/config.ts qui devrait être le seul surchargé désormais.

Modifications proposées

Exemples de surcharges

Changement de la couleur du thème

// file: src/config.ts

import { createTheme } from '@mui/material/styles';
import defaultTheme from './config/theme';

const config: Config = {
  ...
  theme: createTheme(defaultTheme, {
    palette: {
      primary: {
        main: '#00ff00'
      }
    }
  }),
  ...
};

export default config;

Modifications des ressources

// file: src/config.ts

import resources from './resources';
import MyCustomOrganizationEditPage from './custom/MyCustomOrganizationEditPage.tsx';

const overridenResources = { ...resources };

// Exemple 1 - Suppression d'une ressource
delete overridenResources['Idea'];

// Exemple 2 - Changement de l'arborescence des ressources dans le menu
overridenResources['Organization'].config.options.parent = undefined;

// Exemple 3 - Masquer l'onglet Import d'une ressource (n'est actuellement implémenté que pour Event)
overridenResources['Organization'].config.options.importable = false;

// Exemple 4 - Personnaliser l'édition d'une ressource
overridenResources['Organization'].config.edit = MyCustomOrganizationEditPage;

const config: Config = {
  ...
  resources: overridenResources
  ...
};

export default config;

Ajout d'un dataServer

// file: src/config.ts

const config: Config = {
  ...
  dataServers: {
    cdlt: {
      baseUrl: 'https://data.lescheminsdelatransition.org/',
      externalLinks: true,
    },
  }
  ...
};

export default config;

Ainsi, aucun fichier de la structure initiale d'Archipelago n'est modifié.

Je pense que pour une customisation plus poussée, ça ne suffira pas, il faudra soit rajouter quelques composants à customiser (exemple : BaseView, ShowView, etc. mais on pourra tjrs par la suite rajouter des propriétés sans que ce soit breaking.

Si j'ai oublié des propriétés qui sont déjà surchargées dans vos instances, n'hésitez pas à me le dire que je les rajoute dès maintenant.

simonLouvet commented 3 months ago

Je trouve que ca va dans le bon sens de pouvoir configurer/customiser archipelago plus facilement. Si je comprends bien cette méthode permettra de mixer des customisations existantes de l'instance de transiscope nantes qui sont en fichier js avec le noyau archipelago en tsx. Est-ce que j'ai bien compris le besoin @mguihal ?

srosset81 commented 3 months ago

Pour ma part, je ne comprend pas ça:

Hors cette surcharge rend impossible le changement ultérieur de nom des fichiers ainsi modifiés.

Pourquoi le passage en Typescript empêcherait-il de simplement overwriter des fichiers avec des autres ? Sachant que, dans le Dockerfile, le build est fait après avoir fait cette copie...

Si pour vous, mettre toute la config dans un seul fichier semble plus simple, c'est OK pour moi. Mais je m'interroge sur le fait de devoir passer par ce fichier de config pour des composants React comme Layout, AppBar, LoginPage, HomePage... (et la liste peut être infinie en fait).

Pour ces composants, cela semble plus simple de pouvoir simplement les overwriter comme c'est fait actuellement.

mguihal commented 3 months ago

Je trouve que ca va dans le bon sens de pouvoir configurer/customiser archipelago plus facilement. Si je comprends bien cette méthode permettra de mixer des customisations existantes de l'instance de transiscope nantes qui sont en fichier js avec le noyau archipelago en tsx. Est-ce que j'ai bien compris le besoin @mguihal ?

Pourquoi le passage en Typescript empêcherait-il de simplement overwriter des fichiers avec des autres ? Sachant que, dans le Dockerfile, le build est fait après avoir fait cette copie...

Yes, le but était de clarifier et de regrouper la façon dont les surcharges étaient faites, pour éviter les possibles bugs dû au changement d'extension des fichiers s'ils passent en Typescript.

Par exemple :

Si on laisse comme actuellement

On se rend compte que cette version v2.X est donc breaking sur les surcharges, et devrait alors être majeure...

Avec la proposition de cette PR

Si pour vous, mettre toute la config dans un seul fichier semble plus simple, c'est OK pour moi. Mais je m'interroge sur le fait de devoir passer par ce fichier de config pour des composants React comme Layout, AppBar, LoginPage, HomePage... (et la liste peut être infinie en fait). Pour ces composants, cela semble plus simple de pouvoir simplement les overwriter comme c'est fait actuellement.

J'ai repris les composants qui sont inclus dans le fichier App.jsx qui sert de point d'entrée. Ca permet à priori de ne pas avoir à surcharger ce fichier qui peut contenir de la logique supplémentaire (l'optimisation des requêtes introduite il n'y a pas si longtemps par exemple), et être indépendant des changements de React-Admin (par exemple lors d'une prochaine version majeure de RA, l'un de ses paramètres sera peut-être renommé ou supprimé...).

Concernant les composants, Layout, LoginPage et HomePage sont justement dans ce fichier.
Le composant LoginPage n'est actuellement pas surchargeable car directement importé de Semapps, ça nécessite donc actuellement de surcharger ce point d'entrée juste pour surcharger un composant, pas top. Seul les composants Menu et AppBar sont rajoutés ici par simplicité, alors qu'on peut les surcharger via le composant Layout, pour les customisations ne souhaitant pas tout customiser le Layout de l'app de A à Z.

(et la liste peut être infinie en fait)

J'en ai bien conscience, et je n'ai pour l'instant pas de solution toute faite, je proposais juste ce premier jet d'amélioration dans le but de rendre la cohabitation entre les surcharges et les futures versions de l'app non-breaking.

srosset81 commented 3 months ago

OK merci @mguihal je comprend mieux.

C'est un choix à faire entre avoir laisser plus de liberté aux développeurs, ou garantir que leur code ne casse pas. Pour moi si je fais de la surcharge, c'est un peu comme faire un fork, donc j'ai moi-même la responsabilité de m'assurer lors des mises à jour que tout fonctionne bien. Pour le moment je n'utilise pas Archipelago excepté pour celui de l'AV (mais aucun composant n'est surchargé) donc ça m'est un peu égal. Mais je pense que ce serait important que cette surcharge reste dans certaines limites.

En gros pour moi il y a deux possibilités:

A mon avis ce serait une erreur de tout vouloir mettre dans la première catégorie, ça mettrait trop de responsabilité de notre côté. Et donc il faut laisser ouvert la porte à la deuxième possibilité, et puis valider chaque composant qu'on veut inclure dans la config générale pour décider si pour nous c'est vraiment une customisation courante et importante.

Si pour @simonLouvet et toi les composants de cette PR correspondent à cette définition, je suis néanmoins OK.

mguihal commented 3 months ago

C'est un choix à faire entre avoir laisser plus de liberté aux développeurs, ou garantir que leur code ne casse pas. Pour moi si je fais de la surcharge, c'est un peu comme faire un fork, donc j'ai moi-même la responsabilité de m'assurer lors des mises à jour que tout fonctionne bien. Pour le moment je n'utilise pas Archipelago excepté pour celui de l'AV (mais aucun composant n'est surchargé) donc ça m'est un peu égal. Mais je pense que ce serait important que cette surcharge reste dans certaines limites.

Ca revient à mes questionnements dans ce commentaire de l'automne dernier : https://github.com/assemblee-virtuelle/archipelago/pull/137#issuecomment-1753972070

Je n'ai pas la vision pour savoir quel était l'objectif dans la création d'Archipelago dès le début, mais dans tous les cas pour moi la version actuelle sur ce repo ne peut pas être mise en production sans personnalisation, elle est trop générique pour répondre à tous les cas concrets. Et même si on parle de personnalisation minimes genre la couleur du thème ou quelques configs genre les serveurs requêtés, ça ne pourra pas répondre à des besoins plus loin que ceux de l'Assemblée Virtuelle.

Pour moi il y a plusieurs possibilités :

Pour moi ces solutions sont pas forcément compatibles les unes avec les autres (dans un cas on autorise pas mal de customisations ici, dans l'autre on montre juste un design/utilisation possible exemple sans permettre de customiser), donc il faut choisir...

mguihal commented 3 months ago

Si ça peut aider à la réflexion, voici une liste personnelle et à chaud de customisations/changements de design que je vois possible à améliorer dans l'app ces prochains mois :

srosset81 commented 3 months ago

J'ai de la peine à voir la différence entre l'option 1 et 3. Pour moi en tout cas Archipelago n'est pas un framework (option 2): cela impliquerait de devoir écrire toute une documentation pour ça, qui ferait certainement doublon avec SemApps (dont on arrive déjà pas à maintenir la documentation). Dans toutes nos discussions à travers les années, nous n'avons jamais considéré Archipelago comme un framework.

pour moi la version actuelle sur ce repo ne peut pas être mise en production sans personnalisation, elle est trop générique pour répondre à tous les cas concrets. Et même si on parle de personnalisation minimes genre la couleur du thème ou quelques configs genre les serveurs requêtés, ça ne pourra pas répondre à des besoins plus loin que ceux de l'Assemblée Virtuelle.

Tu dis "Elle est trop générique pour répondre à tous les cas concrets" ... mais est-elle assez générique pour répondre à certains cas concrets ? Je ne crois pas à l'application miracle qui résoudrait tous les problèmes de l'humanité. Tu gères d'autres déploiements que Transiscope en Pays Nantains ? Car pour moi les cas d'usage entre cette instance et celle de l'AV sont très proches. L'objectif est de répertorier les projets / acteurs / idées / ressources (PAIR) d'un écosystème donné.

Pour moi Archipelago est intimement lié à l'ontologie PAIR. Quelqu'un qui veut reprendre Archipelago avec des ontologies tout à fait différentes ferait mieux de faire un fork. Ou alors il peut simplement utiliser React-Admin et SemApps. Pas forcément besoin de s'embêter à forker Archipelago.

Pour moi Archipelago est une application complète, qui devrait avoir une ergonomie optimale car elle a pour vocation par être utilisée par toutes sortes de personnes. Dans la liste des améliorations que tu proposes, je pense que toutes ont leur place dans le code d'Archipelago.

Il faut aussi prendre en compte le fait que, lorsque nous aurons des Pods collectifs ("Cods"), d'ici à une année maximum, Archipelago va très certainement devenir une application déployée sur un seule domaine, et qu'on pourra utiliser pour se connecter à un Cod. A ce moment, on pourra envisager certaines customisations, comme la couleur, le logo, le type de ressources utilisées (qui seront enregistrés comme des configurations dans le Cod lui-même), mais on ne pourra pas changer des composants.

Bien sûr il sera toujours possible de faire un fork de l'application, et de la déployer sur son propre domaine. Par exemple pour un client qui aurait des besoins spécifiques et les moyens de faire des développements. Mais comme dit précédemment, ce n'est pas à nous soucier des forks.

simonLouvet commented 3 months ago

J'ai de la peine à voir la différence entre l'option 1 et 3. Pour moi en tout cas Archipelago n'est pas un framework (option 2): cela impliquerait de devoir écrire toute une documentation pour ça, qui ferait certainement doublon avec SemApps (dont on arrive déjà pas à maintenir la documentation). Dans toutes nos discussions à travers les années, nous n'avons jamais considéré Archipelago comme un framework.

pour moi la version actuelle sur ce repo ne peut pas être mise en production sans personnalisation, elle est trop générique pour répondre à tous les cas concrets. Et même si on parle de personnalisation minimes genre la couleur du thème ou quelques configs genre les serveurs requêtés, ça ne pourra pas répondre à des besoins plus loin que ceux de l'Assemblée Virtuelle.

Tu dis "Elle est trop générique pour répondre à tous les cas concrets" ... mais est-elle assez générique pour répondre à certains cas concrets ? Je ne crois pas à l'application miracle qui résoudrait tous les problèmes de l'humanité. Tu gères d'autres déploiements que Transiscope en Pays Nantains ? Car pour moi les cas d'usage entre cette instance et celle de l'AV sont très proches. L'objectif est de répertorier les projets / acteurs / idées / ressources (PAIR) d'un écosystème donné.

Pour moi Archipelago est intimement lié à l'ontologie PAIR. Quelqu'un qui veut reprendre Archipelago avec des ontologies tout à fait différentes ferait mieux de faire un fork. Ou alors il peut simplement utiliser React-Admin et SemApps. Pas forcément besoin de s'embêter à forker Archipelago.

Pour moi Archipelago est une application complète, qui devrait avoir une ergonomie optimale car elle a pour vocation par être utilisée par toutes sortes de personnes. Dans la liste des améliorations que tu proposes, je pense que toutes ont leur place dans le code d'Archipelago.

Il faut aussi prendre en compte le fait que, lorsque nous aurons des Pods collectifs ("Cods"), d'ici à une année maximum, Archipelago va très certainement devenir une application déployée sur un seule domaine, et qu'on pourra utiliser pour se connecter à un Cod. A ce moment, on pourra envisager certaines customisations, comme la couleur, le logo, le type de ressources utilisées (qui seront enregistrés comme des configurations dans le Cod lui-même), mais on ne pourra pas changer des composants.

Bien sûr il sera toujours possible de faire un fork de l'application, et de la déployer sur son propre domaine. Par exemple pour un client qui aurait des besoins spécifiques et les moyens de faire des développements. Mais comme dit précédemment, ce n'est pas à nous soucier des forks.

Je suis d'accord avec la vision de @srosset81 sur archipelago/pair/COD.

J'ai cependant besoin de customiser des composants plus profonds dans beaucoup de projets pour lesquels je suis parti d'un archielago surchargé et je fais régulièrement d'écrasement lourd et des recopie de fichiers massive pour des configurations avancées (logout oidc distant, comportement de dataAdapter, Layouts, composant servant àplusieurs ressources...) car je n'ai pas l'énergie et le temps de passer par le processus contributif de archipelago ou semapps. J'encouragerai donc les initiatives qui tendent à pouvoir plus customiser par configuration (fichier json ou cod), customisation (config.js) ou réutilisation granulaire (exposer plus de composant interne de semapps, voir de archipelago, pour construire son propre code). Cette volonté se traduit dans le plan stratégique de Data Players comme un poste de dépense R&D si on arrive à lever des fonds. @srosset81 répond souvent que je n'ai qu'a forker et faire différemment et je suis d'accord sur le court terme, mais j'aimerais faire plus de config json (datamodel des ressources, dataserver, label des predicat/ressource, themes avancé, ontologies et context, authprovider) et pouvoir reconstruire des composants plus facilement à partir de briques semapps plus granulaire.

Je n'ai pas vraiment besoin de fichier de config que tu proposes @mguihal et j'ai surtout l'impression que c'est une question de devx (rendre la code plus accessible et compressible) et de migration tsx qui n'aura lieu qu'une fois. Je ne suis cependant pas opposé et cela me conviendra très bien d'aller (et de faire) dans ta direction si cela te semble utile/important. Je peux donner un coup de main à la migration tsx de transiscope en pays nantais si cela peut t'aider.

Quel est ton frein @mguihal à faire ces évolutions dans archipelago?

mguihal commented 3 months ago

Eh bien je ne sais pas trop quoi répondre à tout ça...

Si vous ne voyez pas d'utilité à faire ces modifications, alors ne les faisons pas, on continuera de bricoler en surchargeant certains fichiers comme on peut.

Quel est ton frein @mguihal à faire ces évolutions dans archipelago?

Le risque que ça soit trop restreint à une instance/besoin spécifique, et que ça ne plaise pas à tout le monde ici :)

Il faut aussi prendre en compte le fait que, lorsque nous aurons des Pods collectifs ("Cods"), d'ici à une année maximum, Archipelago va très certainement devenir une application déployée sur un seule domaine, et qu'on pourra utiliser pour se connecter à un Cod. A ce moment, on pourra envisager certaines customisations, comme la couleur, le logo, le type de ressources utilisées (qui seront enregistrés comme des configurations dans le Cod lui-même), mais on ne pourra pas changer des composants.

Il y a donc bien une vision à long terme derrière Archipelago (qui mériterait d'être présentée/expliquée quelque part je pense). Mais est-ce que cette vision correspond bien aux besoins ?

srosset81 commented 3 months ago

@mguihal Oui nous en avons parlé lors des 2 dernières résidences de l'AV. La demande de subvention à l'ADEME, qui a été refusée, avait pour but (notamment) d'opérer ce changement d'architecture. Il faudrait un document pour expliciter tout ça, mais comme on est tous bénévoles sur ce projet (et sur des montagnes d'autres projets) c'est pas évident de trouver le temps de le faire.

mguihal commented 2 months ago

Pas de soucis, je comprends.

Je me permets de clôturer cette PR sans la merger du coup