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

Remove outdated CAS #255

Merged
merged 22 commits into from
Sep 5, 2023
Merged

Remove outdated CAS #255

merged 22 commits into from
Sep 5, 2023

Conversation

LeGeek01
Copy link
Contributor

I have done all the following:

  • added the feature
  • added documentation for the feature
  • added tests for the feature
  • ran mypy, black, and unit tests

Suppression des CAS de l'ac Lyon, Geenoble et Clermont-Ferrand car ce mode de connexion a été supprimé du cas Ma classe en Auvergne Rhône Alpes.
La connexion via EduConnect est donc maintenant la seule option de connexion

@LeGeek01
Copy link
Contributor Author

J'en ai profité pour check les ents qui avaient supprimé ça, et j'ai supprimé tout ce qui ne marche plus dans mon dernier commit (après essai de connexion cela retourne bien une erreur)

@bain3
Copy link
Owner

bain3 commented Sep 2, 2023

Hi, thanks for the PR! Can you please also remove ozecollege_yvelines? It looks closed. Also please remove the ENTs from the documentation in docs/source/api/ent.rst. Thanks!

@bain3 bain3 changed the title Delete cas without educonnect Remove outdated CAS Sep 2, 2023
@LeGeek01
Copy link
Contributor Author

LeGeek01 commented Sep 3, 2023

c'est tout bon

Copy link
Owner

@bain3 bain3 left a comment

Choose a reason for hiding this comment

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

There are still some differences between the docs and code (for example ac_rennes is missing in the docs, but is still in the codebase, I didn't check everything yet). pls fix

@LeGeek01 LeGeek01 requested a review from bain3 September 5, 2023 18:02
@LeGeek01
Copy link
Contributor Author

LeGeek01 commented Sep 5, 2023

normalement tout est bon cette fois, je vais recheck les ents pour voir si j'en ai pas oublié

@LeGeek01
Copy link
Contributor Author

LeGeek01 commented Sep 5, 2023

cette fois-ci tout est bon

@Bapt5 Bapt5 mentioned this pull request Sep 5, 2023
4 tasks
@bain3 bain3 requested review from Bapt5 and removed request for bain3 September 5, 2023 18:49
@LeGeek01
Copy link
Contributor Author

LeGeek01 commented Sep 5, 2023

Du coup on en est où ?

@Bapt5
Copy link
Collaborator

Bapt5 commented Sep 5, 2023

Tu dois répondre a mon review ou je te demande ce que tu as fait de l'ENT de Rennes

@Bapt5
Copy link
Collaborator

Bapt5 commented Sep 5, 2023

Penses tu pouvoir résoudre les erreurs qui ne passe pas dans les tests ?
C'est a dire vérifier si les ent existe toujours si oui voir si l'authentification à changer et si besoin mettre à jour l'authentification

@Bapt5
Copy link
Collaborator

Bapt5 commented Sep 5, 2023

@bain3 Yes no problem I will squash. We made too much commit sorry

@LeGeek01
Copy link
Contributor Author

LeGeek01 commented Sep 5, 2023

J'ai check un par un les ents, et tous ceux qui sont listés fonctionnent encore
L'erreur des tests vient d'où ?

@Bapt5
Copy link
Collaborator

Bapt5 commented Sep 5, 2023

J'ai check un par un les ents, et tous ceux qui sont listés fonctionnent encore L'erreur des tests vient d'où ?

Cliques dessus tu vas voir les details je crois qu'il y a l'ent haut de seine et celui de la creuse

@Bapt5
Copy link
Collaborator

Bapt5 commented Sep 5, 2023

Pour être honnête je ne sais pas ce que j'ai foutu au début pour virer l'ac rennes, j'ai surrement pas fait gaffe au complex ent

Oui effectivement il faut faire gaffe il y a beaucoup d'ent, il ne faut pas en oublier et il ne faut pas casser une ent dont peut être des utilisateurs se servaient

@LeGeek01
Copy link
Contributor Author

LeGeek01 commented Sep 5, 2023

Les tests utilisent la version demo de pronote ?

@LeGeek01
Copy link
Contributor Author

LeGeek01 commented Sep 5, 2023

Si oui l'erreur est toute trouvée, l'espace démo n'a pas été mis à jour vers une base 2023

@LeGeek01
Copy link
Contributor Author

LeGeek01 commented Sep 5, 2023

Ouais c'est ça, en gros la dernière période définie sur l'espace démo est terminée depuis les vacances de juillet, et la base n'a pas été mise à jour ce qui explique l'erreur

@LeGeek01
Copy link
Contributor Author

LeGeek01 commented Sep 5, 2023

Normalement quand index fera la mise à jour de la base de démo, il n'y aura plus d'erreur

@Bapt5
Copy link
Collaborator

Bapt5 commented Sep 5, 2023

Les tests utilisent la version demo de pronote ?

Non je parle des tests pour les ENT. Et ceux ci ne vont pas se résoudre avec la mise à jour de la démo

@LeGeek01
Copy link
Contributor Author

LeGeek01 commented Sep 5, 2023

Les tests utilisent la version demo de pronote ?

Non je parle des tests pour les ENT. Et ceux ci ne vont pas se résoudre avec la mise à jour de la démo

Oui je suis en train de regarder, les deux liens fonctionnent par contre j'ai remarqué qu'il manque le sélecteur de login sur l'ent creuse, est-ce que ça viendrai de la ?
Pour l'enc haut de seine je n'ai pas trouvé de problème

@Bapt5
Copy link
Collaborator

Bapt5 commented Sep 5, 2023

Oui je suis en train de regarder, les deux liens fonctionnent par contre j'ai remarqué qu'il manque le sélecteur de login sur l'ent creuse, est-ce que ça viendrai de la ? Pour l'enc haut de seine je n'ai pas trouvé de problème

Non mais le sélecteur de login ne marche pas comme si tu cliquais. Tout se fait avec des requêtes ont imite a aucun moment les cliques d'une souris. Donc quand je fais ça je regarde avec l'onglet Réseau de l'outil de développement de mon navigateur, les requêtes faites par les ENT. Et ainsi je reconstitue ce que font les ENT.
Je n'ai pas le temps de m'y pencher et je sens que tu n'as pas bien l'habitude avec ces méthodes de reverse engineering.

So I suggest, if @bain3 is ok, to comment the code of ent_creuse and enc_hauts_de_seine waiting for someone to fix them

@bain3
Copy link
Owner

bain3 commented Sep 5, 2023

Yeah sure, they don't work anyways...

@LeGeek01
Copy link
Contributor Author

LeGeek01 commented Sep 5, 2023

Oui je suis en train de regarder, les deux liens fonctionnent par contre j'ai remarqué qu'il manque le sélecteur de login sur l'ent creuse, est-ce que ça viendrai de la ? Pour l'enc haut de seine je n'ai pas trouvé de problème

Non mais le sélecteur de login ne marche pas comme si tu cliquais. Tout se fait avec des requêtes ont imite a aucun moment les cliques d'une souris. Donc quand je fais ça je regarde avec l'onglet Réseau de l'outil de développement de mon navigateur, les requêtes faites par les ENT. Et ainsi je reconstitue ce que font les ENT. Je n'ai pas le temps de m'y pencher et je sens que tu n'as pas bien l'habitude avec ces méthodes de reverse engineering.

So I suggest, if @bain3 is ok, to comment the code of ent_creuse and enc_hauts_de_seine waiting for someone to fix them

je dis ça parce que je vois tous les autres ent avec ce sélecteur et il y a que l'ent de creuse qui ne l'a pas
et pour enc hauts de seine j'ai peut-être trouvé le soucis, le code appelle le form de login avec l'id "auth_form" sauf que quand je regarde dans le F12 ce n'est pas "auth-form" mais "kc-form-login", je suppose donc que ça viens de là

@LeGeek01
Copy link
Contributor Author

LeGeek01 commented Sep 5, 2023

si mes hypothèses sont correctes, le test ent ne devrait plus retourner d'erreur

@LeGeek01
Copy link
Contributor Author

LeGeek01 commented Sep 5, 2023

ok donc l'ent creuse est fixé, par contre pourquoi le black formatter se met à fail maintenant

@Bapt5
Copy link
Collaborator

Bapt5 commented Sep 5, 2023

Merci bien vu pour l'ent creuse, c'était exactement ça l'erreur.
C'est bon j'ai rajouté l ent creuse en educonnect et j'ai run black. Et normalement j'ai rajouté la doc pour les fonctions que j'ai mise. Donc on est bon.
Tout est bon pour toi ?

@LeGeek01
Copy link
Contributor Author

LeGeek01 commented Sep 5, 2023

si tout fonctionne on est parfait

@Bapt5 Bapt5 merged commit 2299247 into bain3:master Sep 5, 2023
3 of 4 checks passed
@LeGeek01
Copy link
Contributor Author

LeGeek01 commented Sep 5, 2023

@Bapt5 du coup tu vas faire une nouvelle release pour fixer les ent ?

@bain3
Copy link
Owner

bain3 commented Sep 5, 2023

Perfect, thank you both!

@Bapt5
Copy link
Collaborator

Bapt5 commented Sep 5, 2023

@Bapt5 du coup tu vas faire une nouvelle release pour fixer les ent ?

Je vais laisser @bain3 faire cela

@LeGeek01
Copy link
Contributor Author

LeGeek01 commented Sep 5, 2023

dac pas de soucis

@LeGeek01
Copy link
Contributor Author

LeGeek01 commented Sep 5, 2023

ravi d'avoir pu vous aider :)

@Bapt5
Copy link
Collaborator

Bapt5 commented Sep 5, 2023

Perfect, thank you both!

No problem 😉

@lesensei
Copy link
Contributor

lesensei commented Sep 7, 2023

Bonjour à tous,

Je me permets de poser la question ici, parce que ce n'est pas clair pour moi.

Est-ce que l'ENT des Hauts-de-Seine est censé fonctionner suite à cette PR ?

Le commentaire de @Bapt5 (#255 (comment)) me donne l'impression qu'il a été abandonné (et la mise en commentaire du code dans ent.py semble aller dans ce sens).

Mais dans le même temps, le commentaire de @LeGeek01 (#255 (comment)) laisse entendre qu'il a réglé le problème, et c'est peut-être moi qui ne comprend pas comment me connecter dans ce département désormais, du coup.

S'il faut comme je le crois corriger le code pour que ça fonctionne dans les Hauts-de-Seine, j'essayerai de faire le nécessaire !

Merci :-)

@Bapt5
Copy link
Collaborator

Bapt5 commented Sep 7, 2023

Non il n'a pas été corrigé car la modification de @LeGeek01 ne fonctionnait pas

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.

4 participants