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

[Fiche taxon] Ajout d'un onglet "Médias" et un composant de galerie photo paginée #3229

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

jules-jean-louis1
Copy link

Closes #69854647
Closes #69854746

Backend

Ajout d'une nouvelle route GET /gn_commons/medias/species/ dans gn_commons/medias/.
Cette route permet de récupérer les médias associés à une espèce en utilisant son identifiant cd_nom. La réponse est paginée.

Frontend

Ajout d'un onglet Médias dans la page de la fiche espèce. Le composant DisplayMedia est utilisé pour gérer l'affichage dynamique des différents types de médias (images, vidéos, audios).
Capture d’écran du 2024-10-15 15-56-28

Copy link

codecov bot commented Oct 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.00%. Comparing base (9c792e4) to head (c087a2e).

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3229      +/-   ##
===========================================
+ Coverage    83.97%   84.00%   +0.02%     
===========================================
  Files          122      122              
  Lines         9719     9733      +14     
===========================================
+ Hits          8162     8176      +14     
  Misses        1557     1557              
Flag Coverage Δ
pytest 84.00% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@edelclaux edelclaux force-pushed the feat/spieces-page/add-tab-media branch from ef59a57 to 2781a47 Compare October 17, 2024 10:39
@edelclaux edelclaux force-pushed the feat/spieces-page/add-tab-media branch 2 times, most recently from 2dc0d3c to e702a5a Compare October 24, 2024 15:24
@edelclaux
Copy link
Contributor

Résumé des échangs autour de la route:

Proposition de fonctionnement après réunion technique

Dans l'idéal, faire une route générique dans medias qui récupère les medias associés à un cd_ref.
Le fait que ces médias soit associés à des observations de la synthèse devrait être décrit en paramètre de la requête.

Les permissions devraient pouvoir être gérés en s'appuyant sur has_instance_permissions, ou filter_by_scope.

Si ça devenait un sac de noeud, re-basculer vers la version simpliste de la requête dans la synthese.

Résulat après analyse de NS

Notes route media

Faire une route au niveau des medias: geonature/backend/geonature/core/gn_commons/medias/routes.py

Cette route doit permettre de requêter les medias d'un ou plusieurs module associés à un cd_ref.

L'idée est de rendre cette route agnostique du module en quetsion (synthèse, occtax, etc.), en s'appuyant sur la mécanique existante, BibTablesLocation.

Problèmes rencontrés

BibTablesLocation est incomplète

Problème très mineur. Il suffirait de rajouter une ligne pour la synthèse.

BibTablesLocation n'est pas suffisante

Exemple de récupération des medias dans occtax. Il faudrait récupérer les medias des occurences.

    # Exemple de démo de construction d'un objet dynamiquement, plaqué sur l'objet et la table décrite bib_tables_location. A ce stade, le résultat de la requête dans bib_tables_location est supposé connu.
    field = "unique_id_sinp_occtax"
    table_cor_counting = "cor_counting_occtax"
    t_cor_counting = sa.Table(
        table_cor_counting, metadata, autoload_with=engine, schema="pr_occtax"
    )

    query = (
        select(t_medias)
        .join(
            t_cor_counting,
            t_cor_counting.c[field] == t_medias.c.uuid_attached_row,
        )
    )

Or, le modèle de données dans Occtax est tel que cette requête ne permet pas d'effectuer une sélection sur le cd_nom. Il faut rajouter une jointure sur t_occurrences_occtax.

    # Exemple de démo de construction d'un objet dynamiquement, plaqué sur l'objet et la table décrite bib_tables_location. A ce stade, le résultat de la requête dans bib_tables_location est supposé connu.
    field = "unique_id_sinp_occtax"
    table_cor_counting = "cor_counting_occtax"
    t_cor_counting = sa.Table(
        table_cor_counting, metadata, autoload_with=engine, schema="pr_occtax"
    )

    table_occurences = "t_occurrences_occtax"
    t_occurences = sa.Table(table_occurences, metadata, autoload_with=engine, schema="pr_occtax")

    query = (
        select(t_medias)
        .join(
            t_cor_counting,
            t_cor_counting.c[field] == t_medias.c.uuid_attached_row,
        )
        .join(
            t_occurences,
            t_occurences.c.id_occurrence_occtax == t_cor_counting.c.id_occurrence_occtax,
        )
        .join(Taxref, Taxref.cd_nom == t_occurences.c.cd_nom)
        .where(Taxref.cd_ref == cd_ref)
    )

L'information pour effectuer cette jointure supplémentaire n'est pas contenue actuellement dans bib_table location.

Solution envisagée

Le procédé de jointure de filtrage par cd_ref est propre à la structure du modèle de donnée dans chaque module.

Non seulement la logique portée par la table BibTablesLocations est difficilement transposable à des cas plus complexes, mais ça ne semble en plus vraiment pas souhaitable. On ne souhaite certainement pas exposer au sein de gn_commons la logique de fonctionnement existante au sein des modules, et créer ainsi une connaissance et un couplage qui n'a plus rien à voir avec l'idée d'une route générique.

Il nous a semblé qu'une idée pourrait être de s'appuyer sur une logique plus orientée module.

De la même manière que l'on procède avec les méthodes un peu partagées comme filter_by_scope, ou l'ensemble des méthodes liées à l'import dans ImportActions, les modules pourraient mettre à disposition une interface genre QueryActions / des méthodes d'interactions. C'est évidemment une idée à la fois impactante, et faiblement définie.

Conclusion

Nous ne souhaitons pas intégrer ce niveau d'intrication dans la création d'une route pour récupérer les medias d'un taxon dans la synthèse.
Nous avions convenu que "Si ça devenait un sac de noeud, re-basculer vers la version simpliste de la requête dans la synthese."
Ca y est, c'est un sac de noeud. Nous souhaitons donc maintenir la version simpliste de la requête dans la synthèse :)

@jules-jean-louis1 jules-jean-louis1 marked this pull request as ready for review October 24, 2024 16:10
@camillemonchicourt
Copy link
Member

Quand on affiche un média, il faudrait ajouter ses infos de base (titre, description, auteur...) mais aussi un minimum d'info sur l'observation à laquelle le média est associé (date, commune ?, autre ?).

Attention les médias d'une observation peuvent aussi être des PDF, des audios, des vidéos...
Ils sont affichés aussi ? Ou exclus ?

@edelclaux
Copy link
Contributor

Quand on affiche un média, il faudrait ajouter ses infos de base (titre, description, auteur...) mais aussi un minimum d'info sur l'observation à laquelle le média est associé (date, commune ?, autre ?).

Attention les médias d'une observation peuvent aussi être des PDF, des audios, des vidéos... Ils sont affichés aussi ? Ou exclus ?

Ils sont affichés. On a réutilisé le composant frontend/src/app/GN2CommonModule/form/media/display-medias.component.html

Ok pour rajouter les infos mentionnées. On peut faire la liste ?
Meta-données du media:

  • titre
  • auteur
  • description
  • observation:
    • id ? auteur ? date ? renvoit vers la fiche Observation ?

@camillemonchicourt
Copy link
Member

OK super si les infos du média sont affichés en reprenant le composant fourni par défaut.
Pour les infos sur l'observation, je sais pas trop. J'aurai dit au moins la date, mais on a la date du média en fait déjà, donc on peut mettre de côté ce sujet certainement. Un lien vers l'observation serait utile par contre, bien vu.

@edelclaux edelclaux force-pushed the feat/spieces-page/add-tab-media branch 2 times, most recently from 997ef1f to e1cf7dd Compare January 30, 2025 15:19
@edelclaux edelclaux force-pushed the feat/spieces-page/add-tab-media branch from e1cf7dd to dd99db4 Compare January 30, 2025 15:22
@edelclaux edelclaux force-pushed the feat/spieces-page/add-tab-media branch from dd99db4 to c087a2e Compare January 30, 2025 15:31
@camillemonchicourt camillemonchicourt added this to the 2.16 milestone Jan 30, 2025
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