-
-
Notifications
You must be signed in to change notification settings - Fork 43
Intégration Skolengo - Installation + Auth #84
base: development
Are you sure you want to change the base?
Conversation
Info : Skolengo est activé uniquement en version dev pour l'instant à des fins de test (__DEV__ || false)
Version typesafe de SkolengoCache + ajout de skoapp-prod dans app.json
Renommage pour avoir un seul nom au sein du projet
Car je suis fatigué donc je fais pas gaffe !
A continuer avec scolengo-api
y'a un conflit sur GradeView.js, il faudrait que tu le résoude |
Ne sert actuellement à rien et sert de fichier de base pour créer l'intégration future de skolengo
+ suppression de l'ancienne implémentation Skolengo dans les Settings
GradeView n'est plus utilisé de toute manière, faudrait le virer complètement même |
Copie du message relayé sur le serveur discord : Mise à jour - Intégration Skolengo "Next Gen" :
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A voir si @maelgangloff à quelque chose à ajouter et si l'application fonctionne pour les utilisateurs de Skolengo, mais au moins le code me parait OK !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
J'ai cette erreur
node:events:492
throw er; // Unhandled 'error' event
^
Error: EMFILE: too many open files, watch
at FSWatcher._handle.onchange (node:internal/fs/watchers:207:21)
Emitted 'error' event on NodeWatcher instance at:
at FSWatcher._checkedEmitError (/Users/tom.theret/Documents/git_project/Papillon/node_modules/metro-file-map/src/watchers/NodeWatcher.js:134:12)
at FSWatcher.emit (node:events:514:28)
at FSWatcher._handle.onchange (node:internal/fs/watchers:213:12) {
errno: -24,
syscall: 'watch',
code: 'EMFILE',
filename: null
}
Au vu de ton erreur, c'est ton pc qui doit avoir un probleme ou qui atteint la limite. Je te conseille de supprimer le dossier node_modules et de refaire un |
Je viens de :
ça fonctionne toujours pas 😭 EDIT : Il fallait Watchman sur Mac |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Supprime de app.tsx :
{
name: 'Grade',
component: require('../views/Grades/GradeView').default,
options: {
presentation: 'modal',
headerTintColor: '#fff',
}
},
et fait hyper attention à ta syntaxe
Sans message de votre part, est-ce que la PR est bonne, pouvez-vous la review et pouvez-vous peut être la merge ? |
Pour être validée, une PR doit obtenir deux review favorables. Cette PR est assez longue, ce qui implique d'attendre plus longtemps pour obtenir ces review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pensez à faire un kanban des tickets qui vont découler de cette PR (refresh de token, introspection, vue de notes, etc)
voir un kanban de toutes les tâches liées a Skolengo
disco | ||
).catch(skolengoErrorHandler); | ||
if (!token) return skolengoErrorHandler(); | ||
//console.log({token}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
//console.log({token}); |
return null; | ||
}; | ||
|
||
export const loginSkolengoWorkflow = async ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
faut reformatter, ça manque de lignes vide pour séparer les unités logique
'Skolengo : intégration en cours', | ||
'Veuillez patienter, le processus de connexion à Skolengo à fonctionné.\nMais l\'intégration de Skolengo (NG) n\'est pas encode terminé.\n\nRevenez plus tard.' | ||
); | ||
/* // TODO : Créer l'intégration via scolengo-api |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
faudrait finir au moins l'enregistrement du token et de l'école
@@ -1,36 +1,50 @@ | |||
import AsyncStorage from '@react-native-async-storage/async-storage'; | |||
|
|||
class SkolengoPrivateCache { | |||
class CommonCacheUtils { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Common" mais prefixé de SkolengoCache_ et sous un namespace Skolengo ?
static msToTomorrow() { | ||
const now = Date.now(); | ||
const timePassedToday = now % this.DAY; | ||
const timePassedToday = (now % this.DAY) + this.TIMEZONE_OFFSET; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
C'est du différentiel, laisse tout en UTC0 au lieu de te faire chier avec les fuseaux horaires
@@ -0,0 +1 @@ | |||
--install.ignore-optional true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ne pas mélanger les package managers
).catch(skolengoErrorHandler); | ||
if (!token) return skolengoErrorHandler(); | ||
//console.log({token}); | ||
Alert.alert( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pas besoin de faire de joli toasts pour ce qui restera en develop
export class SkolengoDataProvider { | ||
constructor() { | ||
if (!SkolengoNotImplementedWarn) { | ||
console.warn('SkolengoDataProvider is not implemented'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pas besoin de faire de joli warnings pour ce qui restera en develop
> | ||
<StatusBar | ||
animated | ||
barStyle={UIColors.dark ? 'light-content' : 'dark-content'} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
peut être à généraliser dans le composant
|
||
const selectOption = (index) => { | ||
const selectOption = (index:number) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const selectOption = (index:number) => { | |
const selectOption = (index: number) => { |
Salut, moyen de régler les conflits et de fermer vos discussions si elles sont résolues svp ? |
mais visiblement y a des trucs à modif |
La résolution des conversations est prioritaire sur les conflits qui pourront être gérés dans un second temps afin d'avoir une bonne base.
Cette PR est assez conséquente, il ne faut pas négliger les avis de chacun. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Je suis utilisateur de skolengo et l'authentification fonctionne bien de mon côté.
@NonozgYtb il va falloir fixer les conflits et regarder les remarques de vinceh21, les passer en résolu si possible |
Des news ? |
Je sais pas, faut voir avec "l'équipe Papillon", qui s'occupent de la V7 d'un projet ("collaboratif" "open source") "en interne"... |
Le projet a toujours été comme ça, les nouvelles versions de l'application ont toujours été dev en petit comité puis open à tout le monde... |
Je n'ai pas dit que c'était nouveau 😉 |
Oui, attend la V7 ! Normalement, le système pour les API sera beaucoup plus simple et pratique ! |
Warning
La review de @maelgangloff est très souhaitable.
Checklist d'avant pull request
development
(le cas contraire préciser laquelle)TODO
(aka des annotations pour du code manquant) dans vos modificationsExplications des changements
Différents rajouts liés à la future intégration de Skolengo NG (Nouvelle génération - avec scolengo-api@3)
Certains fichiers du "squelette" de l'application ont été modifiés afin de remplacer l'ancienne intégration (sans
scolengo-api
).Enfin d'autres modifications mineurs ont été apportés (renommage et ajout de fonctionnalités mineures (activation conditionnelle de Skolengo) notamment).
Changelogs proposés
scolengo-api
à la page de recherche d'étab.SelectService
est d'avantage modulaireNote
Pour que votre pull-request soit fusionnée, vous devez obtenir l'approbation de au moins deux membres de l'équipe dont un coordinateur.
Soyez donc patient :)
Informations supplémentaires
Plus d'information dans le titre (et occasionnellement la description) des commits.
Ce merge n'est pas urgent, mais personnellement, je me baserai sur celui-ci pour le PR N°2 sur SkolengoNG.
Tip
N'hésitez pas à revenir vers moi par ce PR ou sur Discord concernant ce Pull Request ou plus globalement sur l'intégration SkolengoNG