Only lead climbs #9
Conversation
…alisée au lieu du niveau de la section la plus facile.
There was a problem hiding this comment.
Hello @pectum83
Encore désolé pour le temps de relecture : /
Voici mes commentaires, n'hésite pas si tu as des questions
# Conflicts: # app/controllers/api/v1/current_users_controller.rb
Updated the handling of the 'only_lead_climbs' parameter across various controllers to ensure consistent behavior by casting the parameter to boolean using ActiveModel. Adjusted schema character set from utf8mb4 to utf8mb3 for multiple tables, optimizing compatibility. Added debug logs and comments within methods related to climbing figures and charts to facilitate future debugging and enhancements.
Introduce 'second' scope to filter ascents based on top rope and multi-pitch second roping statuses. This enhancement improves query flexibility for retrieving specific ascent types.
Introduce a unified filter system using `CragAscentFilter` for ascent data, replacing individual filtering logic in controllers and models. This simplifies the codebase, enhances reusability, and improves maintainability of climbing data filters and analytics.
Streamline and centralize filtering logic through the new `CragAscentFilters` class, replacing scattered filter handling. Simplify API endpoints, models, and services by delegating responsibilities to the updated filtering mechanism, improving maintainability and performance.
This reverts commit 10b0e8e.
|
Salut @lucien-chastan, Désolé pour les fausses manip sur la PR mais je n'ai pas l'habitude de gilthub. J'espère que ça ne t'as pas trop spammé. J'ai intégré tes remarques précédentes. L'intégration des climbing_type en filters a finalement conduit a pas mal de modif. Au passage comme je devais refactoré pas mal de choses j'en ai profité pour homogénéiser certaines parties qui faisaient la même chose de deux manières différentes. J'espère que je n'ai rien cassé. J'ai fait pas mal de tests mais ma base de donnée est toute petite et je ne vois pas les éventuels problèmes de latence. Les points qui restent en attente (et que je ferai avant merge selon ce que tu décides):
Vue l'ampleur des changements je pense qu'il faudra une autre passe de commentaires et de reprise de code pour converger vers quelque chose qui te corresponde. J'ai essayé d'être homogène avec le reste du code mais c'est pas évident de savoir comment tu aurais fait les choses. N'hésites pas à faire des remarques et je modifierai. Christophe. |
|
Hello @pectum83 ! Quand je vois ce développement je me dis qu'il y a quelque chose à faire ! Mais qui va demander un peu plus de travail 😬 En fait avoir un bouton 'uniquement à vue' je me dis : "et si je veux voir uniquement mes flashs ?" Un autre sujet, et c'est là où ça va plus loin, depuis un moment je me dis que c'est dommage de pouvoir choisir le type d'escalade (bloc, voie, grand voie, etc.) uniquement sur le liste, et par sur les chiffres ni sur Analyticks. Donc je te propose d'aller un peu plus loin 😅 Voici ce que j'ai en tête :
Avant les onglets, on ajoute une boîte que l'on peu déplier avec le bouton (FILTRER MON CARNET) Ce qui serait bien c'est que les filtres soit passé en paramètres des liens des onglets en dessous, pour garder les filtrer entre les différentes page. Tu en pense quoi ? C'est fait vite fait, il y a des choses à améliorer en terme d'affichage, c'est dans l'esprit (ça risque de poser des questions sur la page 'Tick list', 'Projet' et "ma carte" qui sont un peu différentes ...) |
|
Ca me plait, je regarde ça. |
|
Cool ! Le LocalStorage ça marche aussi ! Le bouton reste bonne idée 👍 Désolé de rajouter des choses à faire sur cette PR que j'ai mis temps de temps à relire ! |
|
Voila une nouvelle version generalisée des filtres. N'hesites pas à me dire si tu veux des amélioration d'aspect ou de confirmité de code. |
…avec outdoor_controller api
app/models/ascent.rb
Outdated
| scope :by_ascent_statuses, ->(ascentStatusList) { where(ascent_status: ascentStatusList) } | ||
| scope :by_roping_statuses, ->(ropingStatusList) { where(roping_status: ropingStatusList) } | ||
| scope :by_climbing_types, ->(climbingTypesList) { joins(:crag_route).where(crag_routes: { climbing_type: climbingTypesList }) } | ||
| scope :by_ascent_statuses, ->(ascentFilter) { where(ascent_status: ascentFilter) } |
There was a problem hiding this comment.
du coup snake_case ici 😅
There was a problem hiding this comment.
Ah oui. C'est fait.
| @@ -127,12 +122,8 @@ | |||
| namespace :log_books do | |||
| resources :outdoors, only: %i[] do | |||
There was a problem hiding this comment.
Dans la mesure on on a factorisé le log_books pour le current user et les users en ajoutant un parametre user_id, il serait plus propre de déplacer le namespace log_books ailleurs que dans current_user. Qu'en penses tu ?
There was a problem hiding this comment.
Je ferais ça dans un second temps, s'il y a besoin, c'est plus simple et plus sécurisé que tous ce qui touche à son compte passe par current_users
Par contre il va falloir la route
users/:user_uuid/log_books/outoors/stats sinon la vision d'un carnet de croix d'un autre grimpeur ne fonctionnera plus, ou alors il faut laisse les routes dans users/ si on ne fait pas la refacto
There was a problem hiding this comment.
En fait j’ai déjà factorisé le back en ajoutant le test du user dans outdoorcontroller (ligne 84). Du coup le front des users appelle outdoorcontroller avec le user en param. C’est pour ça que je proposais de sortir la route outdoor du current user.
There was a problem hiding this comment.
ha merde désolé je n'avais pas vu que tu avais refactorisé pour pouvoir appeler le carnet de croix d'un autre user depuis la route /current_users/log_books/outdoors/stats.json
Ça ne me semble pas être une bonne chose, ça brise la logique que current_user c'est forcément le user connecté, et users c'est les autres
Et maintenant que j'y pense ça serai étrange de pouvoir filtrer le carnet de croix d'une autre personnes, comme on ne peux pas voir toutes les infos des autres (comme la carte d'escalade, les tick list, l'analytics, etc.)
je préfère qu'il y ai une route dans users qui initialise le module de stats avec des filtres par défait qu'un user ne peux pas changer
En fait j'ai l'impression que la partie carnet de croix d'un autre user pourrais ne pas changer ?
Peut-être juste passer des filtres par défaut dans l'initialisation sans lancer la possibilité de les changer
|
ok je vais supprimer les filtres sur les users. |
|
C'est fait. J'ai juste laissé le filtre "climbing type" dans les autres users pour rester identique a ce qu'il y a vait avant. Pour les autres users je n'ai pas fait de local storage. Ca reset le filtre climbing types a tous les types à chaque refresh. |
|
Normalement ca devrait être tout bon api et app |
|
Dernière retouche : finalement j'ai supprimé les uniq(crag_id) dans les charts et les figures. Avec les filtres complets il suffit de ne pas selectionner "répetition" et "enchainé" |
|
Parfait ! |
Evolution liée au ticket "croix en tete et/ou en moulinette oblyk/oblyk-app#59" pour les deux points.
1 - Permet de n'afficher que les croix en tête avec un selecteur pour ceux qui veulent voir aussi les moulinettes.
2 - Ne compte pas les répétitions d'une même voie dans les croix
Corrige également :
A combiner avec la pull request sur oblyk-app