Skip to content

Conversation

@xavierleune
Copy link
Member

  • Ajout des themes sur la page programme
  • Démo (retours pris en compte: padding sur les descriptions + longueur max)
  • Ajout d'un prompt de suggestion pour générer la description

Cette feature est optionnelle: tant que l'option "Activer les thèmes" n'est pas cochée sur l'édition d'un event, les thèmes ne sont pas pris en compte à l'affichage. Cela permet de ne pas toucher au passé, mais également de travailler en 2 temps sur les futurs thèmes: mise en place du programme rapide puis ajout des thèmes (par exemple).

Important: la PR sur event doit être passée d'abord: afup/event#34

@xavierleune
Copy link
Member Author

Si quelqu'un a une idée pour rector, je sais pas trop pourquoi il prend pas le .env correctement ?

@xavierleune
Copy link
Member Author

Pour résumer concernant .env, j'avais l'erreur suivante lors du lancement de rector dans la CI (uniquement):

cp .env.dist .env
php bin/console cache:warmup --env=dev

In EnvVarProcessor.php line 221:
                                                    
  Environment variable not found: "DATABASE_NAME".  
                                                    

make: *** [Makefile:199: var/cache/dev/AppKernelDevDebugContainer.xml] Error 1

Cela était provoqué par la nouvelle entité EventTheme et son attribut:
#[Table(name: 'afup_conference_theme', connection: 'main', database: '%env(DATABASE_NAME)%', repository: EventThemeRepository::class)]

J'ai cru que le problème venait d'un mauvais chargement du fichier .env, en réalité dotenv n'était pas du tout installé ni configuré, il y avait une manip qui datait dans config.php qui devait faire à peu près pareil mais ne permettait pas au %env(PARAM)% de fonctionner. Au passage le commentaire qui incriminait ting était sans doute un peu à côté 😛

Du coup j'en ai profité pour ajouter symfony/dotenv et l'initialiser, on pourrait probablement prévoir d'utiliser symfony/runtime également qui évite ce genre de manips, mais c'est assez pour aujourd'hui !

Copy link
Contributor

@Mopolo Mopolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quelques derniers trucs, le reste est ok pour moi.

Y'a pas mal de tests behat qui ne passent pas, sûrement un soucis global.

// Handle AJAX requests for updating data
if ($request->isXmlHttpRequest()) {
return $this->handleAjaxRequest($request, $event);
} elseif ($request->getMethod() === 'POST' && $request->request->has('delete')) {
Copy link
Contributor

@Mopolo Mopolo Aug 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Il y a un return dans le if au dessus donc pas besoin de else.

Suggested change
} elseif ($request->getMethod() === 'POST' && $request->request->has('delete')) {
}
if ($request->getMethod() === 'POST' && $request->request->has('delete')) {

@xavierleune xavierleune force-pushed the feature/thematisation branch from ab0e620 to f8a1bca Compare August 7, 2025 12:31
port: "%database_port%"
user: "%database_user%"
password: "%database_password%"
port: "%env(int:DATABASE_PORT)%"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

est-ce que ce genre de changement technique ne serait pas à faire dans une PR dédiée pour ne déployer que ça, suivre le changement en prod, puis ensuite livrer en prod la fonctionnalité, mais bien dissocier les deux pour réduire les risques au déploiement / avoir un meilleur suivi) ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comme la fonctionnalité ne s'active pas par défaut, on peut envisager de faire la mep globale sans activer la feature, pour ne rien casser en cas de rollback, qu'en dis-tu ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

poke @agallou ^^

@stakovicz
Copy link
Contributor

@xavierleune Il y a toute une partie lié à la modification de la configuration.
Est-ce que c'est possible de sortir ça de ta PR ?

C'est pas le meilleur moment pour le moment pour changer un truc aussi sensible 😬

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.

5 participants