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

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

Closed
wants to merge 1 commit into from

Conversation

mguihal
Copy link
Collaborator

@mguihal mguihal commented Apr 9, 2024

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

  • Déplacement du fichier src/config/config.ts vers src/config.ts afin de renforcer l'importance de ce fichier dans la structure, et d'éviter la création d'un dossier inutile dans le répertoire de surcharge.

  • Ajout d'une propriété requise title contenant le titre de l'app

  • Ajout d'une propriété requise theme contenant le thème de l'app

  • Ajout d'une propriété requise resources contenant les resources gérées par l'app

  • Ajout d'une propriété optionnelle dataServers listant les dataServers additionnels

  • Ajout d'une propriété optionnelle Layout pour surcharger le composant de layout par défaut d'Archipelago

  • Ajout d'une propriété optionnelle Menu pour surcharger le composant de menu par défaut d'Archipelago

  • Ajout d'une propriété optionnelle AppBar pour surcharger le composant d'appBar par défaut d'Archipelago

  • Ajout d'une propriété optionnelle HomePage pour surcharger le composant de HomePage par défaut d'Archipelago

  • Ajout d'une propriété optionnelle LoginPage pour surcharger le composant de LoginPage par défaut d'Archipelago

  • Suppression de la propriété importableResources qui est déplacée vers la configuration des ressources (options.importable)

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.

@mguihal mguihal force-pushed the feat-ImproveConfiguration branch 4 times, most recently from ba08786 to d668a0f Compare April 9, 2024 21:51
@mguihal mguihal marked this pull request as ready for review April 9, 2024 22:06
@mguihal mguihal self-assigned this Apr 9, 2024
@simonLouvet
Copy link
Contributor

simonLouvet commented Apr 18, 2024

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
Copy link
Contributor

srosset81 commented Apr 19, 2024

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
Copy link
Collaborator Author

mguihal commented Apr 20, 2024

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

  • Dans mon instance, je surcharge le fichier /resources/Agent/Actor/Organization/OrganizationShow.jsx
  • Dans quelques semaines/mois, une prochaine version v2.X d'Archipelago intègre la conversion de ce fichier en /resources/Agent/Actor/Organization/OrganizationShow.tsx
  • Sur mon instance, je me retrouve donc si je ne fais pas attention, après la mise-à-jour, avec un fichier en double /resources/Agent/Actor/Organization/OrganizationShow.jsx et /resources/Agent/Actor/Organization/OrganizationShow.tsx.
    Et quand le fichier frontend/src/resources/Agent/Actor/Organization/index.js fait import OrganizationShow from './OrganizationShow';, lequel importe-t'il ? Le .jsx ou .tsx ? Ca crée de la confusion et un risque de bugs accru.

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

  • Je dois faire la conversion en Typescript de mon fichier customisé pour que ça continue de fonctionner (mais peut-être que je n'en ai pas envie car trop complexe)

Avec la proposition de cette PR

  • Dans mon instance, je crée le fichier /customPath/.../OrganizationShow.jsx et l'intègre via le fichier /config.ts comme expliqué
  • Dans quelques semaines/mois, une prochaine version v2.X d'Archipelago intègre la conversion de ce fichier en /resources/Agent/Actor/Organization/OrganizationShow.tsx
  • Après MAJ, mon instance ignore ce nouveau fichier, et continue d'utiliser mon fichier surchargé. La v2.X n'est donc pas breaking. Je ne suis pas obligé de convertir mon fichier customisé en Typescript si je n'en ai pas envie.

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
Copy link
Contributor

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:

  • Tu veux ajouter quelques configurations, comme les couleurs ou les serveurs requêtés, dans ce cas on te propose des configs claires et on garantit que ça ne va pas casser (sauf si BC explicite).
  • Tu as envie de customiser complètement Archipelago pour des besoins très spécifiques. Dans ce cas tu peux faire un fork ou utiliser la méthode de la surcharge, mais c'est à toi de t'assurer lors des mises à jour que tes modifications fonctionnent toujours.

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
Copy link
Collaborator Author

mguihal commented Apr 22, 2024

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 : #137 (comment)

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 :

  • 1/ Soit on voit Archipelago comme une application de base, surcouche à Semapps et React-admin, qui demande à être personnalisée pour être utilisée ensuite (direction actuelle du projet il me semble). Les utilisateurs devront donc surcharger l'application pour l'utiliser, et celle-ci permettre un large choix de customisations possibles pour répondre à tous les cas.

  • 2/ Soit on voit Archipelago comme un framework, surcouche à Semapps et React-admin. Il faudrait pour ça transformer Archipelago en lib publiée sur npm, et les utilisateurs devront par exemple créer une app en important Archipelago un peu de la même façon dont on importe React-admin. Ca résoudrait le souci de fichiers à surcharger, mais ça demanderait pas mal de changements ici...

  • 3/ Soit on voit Archipelago comme une application-exemple / template, en mode "Regardez ce qu'on peut faire avec Semapps et React-admin", et orienter les utilisateurs sur une recopie complète du repo (ou un fork) pour leur projet. On pourrait éventuellement créer un utilitaire du style de create-react-app, en mode npx create-archipelago-app, qui faciliterait la mise en place initiale. Ca résoudrait le souci de fichiers à surcharger aussi.

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
Copy link
Collaborator Author

mguihal commented Apr 22, 2024

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 :

  • Uniformisation des formulaires d'ajout/édition pour les passer sur une seule page (comme ce qui avait été fait pour les évènements)
  • Rendre le design de l'app mobile-first de façon à bénéficier de toutes les fonctionnalités desktop en vue mobile (filtres, recherche, etc.)
  • Amélioration des filtres de façon à rendre ça plus ergonomique et visuel (comme Gogocarto par exemple)
  • Amélioration du menu (revoir son positionnement sur desktop comme sur mobile, simplifier son arborescence pour qu'il soit plus user-friendly)
  • Se questionner sur la pertinence de chacune des ressources proposées
  • Modification de la recherche en haut qui est actuellement contre-intuitive
  • Modification des pages de présentation des organisations pour qu'elles soient plus jolies visuellement (image en bannière, toutes les infos en sideBar mise en dessous et plus discrètement, rajouter un agenda des évènements sur cette page, etc.)
  • Revoir les permissions qui sont mal comprises (permission "enrichir" qui pose plus de problèmes qu'autre chose par exemple)
  • Amélioration de la page de login pour y ajouter des informations sur les providers de connexion, etc.
  • Amélioration des performances des requêtes (pagination, filtrage)
  • Amélioration du SEO en mettant juste le nom des ressources dans les urls et non tout son id
  • Création d'urls et pages personnalisées pour des pages précises comme "Mon profil" ou "Mes paramètres"
  • ...

@srosset81
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Collaborator Author

mguihal commented Apr 24, 2024

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
Copy link
Contributor

@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
Copy link
Collaborator Author

mguihal commented May 6, 2024

Pas de soucis, je comprends.

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

@mguihal mguihal closed this May 6, 2024
@srosset81 srosset81 deleted the feat-ImproveConfiguration branch May 7, 2024 09:45
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

Successfully merging this pull request may close these issues.

3 participants