payutc / server

Web Service exposant l'ensemble des opérations de payutc
9 stars 16 forks source link

Feature CATALOG Service et modifications de getFundations dans ServiceBase #328

Closed roddehugo closed 10 years ago

roddehugo commented 10 years ago

Suite à la première PR j'ai fait des modifications dans le ServiceBase pour y ajouter principalement la méthode checkFundationIds qui retourne le tableau $fun_ids correspondant. Cette fonction était appelée deus fois dans le service GESARTICLE et deux fois dans CATALOG. Bad.

Du coup on passe les paramètres relatifs au checkRight directement dans les paramètres de checkFundationIds qui les transmet à getFundations si fun_ids est NULL (ce qui est le cas ici car depuis l'interface du site internet vitrine du picasso, on se connecte avec une clé d'application mais on ne connait pas nécessairement l'ID de la Fundation PICASSO). De ce fait en faisant appel à getFundations, on appel la méthod checkRight avec les arguments trimbalés depuis.

Il faudrait vérifier avec les tests que les modifications de checkRight ne casse pas le reste du code !

La bise

mattgu74 commented 10 years ago

Tu n'étais pas obligé de faire une nouvelle PR, le simple fait de push dans ta branche met a jour la PR.

D'ailleurs outre les commentaires que j'ai pu faire, il reste toujours le problème de ta branche pas à jour, qui n'est donc pas mergeable tu dois faire ça sur ton ordi: git checkout master git pull payutc master (chez moi le repo principal s'appelle payutc, je sais pas comment c'est chez toi) git checkout feature-catalog git rebase master (si le rebase marche pas, tente le git merge master) git push

roddehugo commented 10 years ago

C'est ce que j'ai fait et il me dit que mon repo forké roddehugo/server is already up to date.

roddehugo commented 10 years ago

Commentaires inutiles supprimés

feuloren commented 10 years ago

git remote -v te donne quoi ?

roddehugo commented 10 years ago

git remote -v

origin git@github.com:roddehugo/server.git (fetch)

origin git@github.com:roddehugo/server.git (push)

Hugo Rodde, Élève Ingénieur à l'Université de Technologie de Compiègne. Branche Génie Informatique 06 27 22 93 47

2014-03-04 21:40 GMT+01:00 Florent Thévenet notifications@github.com:

git remote -v te donne quoi ?

— Reply to this email directly or view it on GitHubhttps://github.com/payutc/server/pull/328#issuecomment-36671388 .

feuloren commented 10 years ago

Ah, il te manque le dépôt "principal" payutc/server Tu peux faire un git remote add upstream git://github.com/payutc/server.git (en remplaçant éventuellement upstream par payutc si ça t'inspire plus) et là tu pourra faire le git pull upstream master et merger dans ton master.

(Travailler avec des forks est assez galère au début)

roddehugo commented 10 years ago

Okay en effet je comprend mieux

Hugo Rodde, Élève Ingénieur à l'Université de Technologie de Compiègne. Branche Génie Informatique 06 27 22 93 47

Le 4 mars 2014 21:58, Florent Thévenet notifications@github.com a écrit :

Ah, il te manque le dépôt "principal" payutc/server Tu peux faire un git remote add upstream git:// github.com/payutc/server.git (en remplacement éventuellement upstream par payutc si ça t'inspire plus) et là tu pourra faire le git pull upstream master et merger dans ton master.

(Travailler avec des forks est assez galère au début)

— Reply to this email directly or view it on GitHubhttps://github.com/payutc/server/pull/328#issuecomment-36674348 .

roddehugo commented 10 years ago

Salut, suite au précédent merge de la branche master j'ai cette erreur dans les logs : ALERT: Fatal Error (E_ERROR): Cannot access protected property Slim\Slim::$request {"file":"/...../payutc/server/src/Payutc/Dispatcher/Json.php","line":40}

mattgu74 commented 10 years ago

En mergeant tu as récupéré les dernières modifs de payutc, notamment cette PR : https://github.com/payutc/server/pull/302

Qui vient protéger l'API en bloquant les requêtes GET l'essentiel des appels doivent se faire en POST.

Si tu as des méthodes qui justifie le fait d'être appelé en GET, tu peux autoriser l'appel en ajoutant tes méthodes dans Payutc\Mapping\Services::$servicesGET

https://github.com/trecouvr/server/blob/b7f1a76f4b3cd47d6cb06fd7a3086d79792459be/src/Payutc/Mapping/Services.php#L21

mattgu74 commented 10 years ago

Sinon bonne nouvelle, travis est content et github d'accord pour merger ;)

Je vais relire encore une fois ton code, vu que jusque la j'ai plutôt survolé, mais je peu d'hors et déjà te dire que je (ou si je le fais pas @trecouvr) risque de demander quelques petits tests unitaires. Permettant d'assurer que les prochaines modifs de code ne puissent pas casser le fonctionnement de ton service.

roddehugo commented 10 years ago

C'est pas de ma faute ça :p mais je vais corriger ça

roddehugo commented 10 years ago

Par contre je ne n'arrive pas à résoudre mon erreur fatale d'avant. Voici le log en entier. C'est lors de l'appel à getStatus, loginApp, etc et elle est bien faite en POST.

[2014-03-07 16:52:51] ALERT: Fatal Error (E_ERROR): Cannot access protected property Slim\Slim::$request {"file":"/Users/hugo/Sites/payutc/server/src/Payutc/Dispatcher/Json.php","line":40} {"uid":"c5cb2c2fccad14646b62b8c8f3d70baa","user":"NULL","application":"NULL","session_id":"g6poqnvglat4vf308pemokrsq4","file":"NULL","line":"NULL","class":"NULL","function":"NULL","args":"NULL","url":"/payutc/server/web//CATALOG/getProductsByCategories","ip":"::1","http_method":"POST","server":"localhost","referrer":"NULL"}

mattgu74 commented 10 years ago

Hum @trecouvr, tu appelles des property protected depuis src/Payutc/Dispatcher/Json.php","line":40

Je suppose qu'une mise a jour de Slim ou une version de PHP un peu plus carré que la tienne impact hugo...

Tu peux nous corriger ça ?

@roddehugo, aucun soucis avec ton code à première vue, on va demander à @trecouvr de nous corriger ça.

roddehugo commented 10 years ago

D'acc

Hugo Rodde, Élève Ingénieur à l'Université de Technologie de Compiègne. Branche Génie Informatique 06 27 22 93 47

Le 7 mars 2014 17:00, Matthieu Guffroy notifications@github.com a écrit :

Hum @trecouvr https://github.com/trecouvr, tu appelles des property protected depuis src/Payutc/Dispatcher/Json.php","line":40

Je suppose qu'une mise a jour de Slim ou une version de PHP un peu plus carré que la tienne impact hugo...

Tu peux nous corriger ça ?

@roddehugo https://github.com/roddehugo, aucun soucis avec ton code à première vue, on va demander à @trecouvr https://github.com/trecouvr de nous corriger ça.

— Reply to this email directly or view it on GitHubhttps://github.com/payutc/server/pull/328#issuecomment-37037346 .

trecouvr commented 10 years ago

Je pourrai rien faire avant lundi donc si entre temps vous trouvez la source du bug hesitez pas a corriger :)

roddehugo commented 10 years ago

Voici le StackTrace dans les logs du server payutc [2014-03-13 09:42:15] ALERT: Fatal Error (E_ERROR): Cannot access protected property Slim\Slim::$request {"file":"/Users/hugo/Sites/payutc/server/src/Payutc/Dispatcher/Json.php","line":40} {"uid":"88ba31ac8877ac4160e5ce68c82692df","user":"NULL","application":"NULL","session_id":"cong39e8p878si3m8dgr5l3c95","file":"NULL","line":"NULL","class":"NULL","function":"NULL","args":"NULL","url":"/payutc/server/web//CATALOG/getStatus","ip":"::1","http_method":"POST","server":"localhost","referrer":"NULL"} [2014-03-13 09:42:15] ALERT: Fatal Error (E_ERROR): Cannot access protected property Slim\Slim::$request {"file":"/Users/hugo/Sites/payutc/server/src/Payutc/Dispatcher/Json.php","line":40} {"uid":"c206cc7efe7eed5416c35195db3864f9","user":"NULL","application":"NULL","session_id":"cong39e8p878si3m8dgr5l3c95","file":"NULL","line":"NULL","class":"NULL","function":"NULL","args":"NULL","url":"/payutc/server/web//CATALOG/loginApp","ip":"::1","http_method":"POST","server":"localhost","referrer":"NULL"} [2014-03-13 09:42:16] ALERT: Fatal Error (E_ERROR): Cannot access protected property Slim\Slim::$request {"file":"/Users/hugo/Sites/payutc/server/src/Payutc/Dispatcher/Json.php","line":40} {"uid":"2a76c9b44c20bdd5e91c85d7a163666d","user":"NULL","application":"NULL","session_id":"cong39e8p878si3m8dgr5l3c95","file":"NULL","line":"NULL","class":"NULL","function":"NULL","args":"NULL","url":"/payutc/server/web//CATALOG/getProductsByCategories","ip":"::1","http_method":"POST","server":"localhost","referrer":"NULL"}

Enfin ce n'est pas vraiment stacktrace mais il n'y a que ça dans les logs

mattgu74 commented 10 years ago

Personellement j'ai récupéré ta branche, et lordsque j'appelle ta méthode, j'obtient cet erreur (qui est normal).

{ error: { type: "Payutc\Exception\CheckRightException" code: 0 message: "Vous devez connecter une application ! (method loginApp)" }- }

Donc sans aller jusqu'à dire que ton service fonctionne parfaitement, pour moi au niveau de la relecture du code ça à l'air bon. Et quand j'utilise le code, je n'ai pas ton erreur chelou... Je t'invite donc à réinstaller ton environnement local payutc, y'a surement une merde quelque part. Essaie aussi de nous donner si possible, ta version de PHP ainsi que la version de Slim que composer a installé dans ton dossier, je pourrais comparer avec ce que j'ai... Peut être que le problème vient de la...

Autre truc à tenter, si tu appelles une méthode "bidon", genre getCasURL, est-ce que ça marche ? Sur ton service ? avec ton code mais sur un autre service ? quand tu reviens sur master ?

trecouvr commented 10 years ago

Faut peut être que tu log une application d'abord matthieu ?

Le 15 mars 2014 03:03, Matthieu Guffroy notifications@github.com a écrit :

Personellement j'ai récupéré ta branche, et lordsque j'appelle ta méthode, j'obtient cet erreur (qui est normal).

{ error: { type: "Payutc\Exception\CheckRightException" code: 0 message: "Vous devez connecter une application ! (method loginApp)" }- }

Donc sans aller jusqu'à dire que ton service fonctionne parfaitement, pour moi au niveau de la relecture du code ça à l'air bon. Et quand j'utilise le code, je n'ai pas ton erreur chelou... Je t'invite donc à réinstaller ton environnement local payutc, y'a surement une merde quelque part. Essaie aussi de nous donner si possible, ta version de PHP ainsi que la version de Slim que composer a installé dans ton dossier, je pourrais comparer avec ce que j'ai... Peut être que le problème vient de la...

Autre truc à tenter, si tu appelles une méthode "bidon", genre getCasURL, est-ce que ça marche ? Sur ton service ? avec ton code mais sur un autre service ? quand tu reviens sur master ?

Reply to this email directly or view it on GitHubhttps://github.com/payutc/server/pull/328#issuecomment-37712937 .

mattgu74 commented 10 years ago

Si tu regardes les lignes d'erreur qu'hugo à copié/collé, tu verras qu'il n'a ni user, ni application de logué.

apuyou commented 10 years ago

Pour la version de Slim, tu peux voir ça dans ton fichier composer.lock.

mattgu74 commented 10 years ago

Bon j'ai testé en faisant un loginApp avant, en donnant les droits à l'appli concerné. Et pour ce qui est de la méthode getProductsByCategories j'ai obtenu un retour avec les catégories et leurs produits. Pour ainsi dire ça à l'air nikel.

Avant de merger, ce serait bien que tu puisses tester quand même, donc je t'invites à comprendre ce qui ne va pas dans ton install. Et dès que tu nous donnes ton feu vert sur le fait que ton code marche comme ce à quoi tu t'attends je mergerais.

trecouvr commented 10 years ago

Weirdos Le 15 mars 2014 16:02, "Matthieu Guffroy" notifications@github.com a écrit :

Bon j'ai testé en faisant un loginApp avant, en donnant les droits à l'appli concerné. Et pour ce qui est de la méthode getProductsByCategories j'ai obtenu un retour avec les catégories et leurs produits. Pour ainsi dire ça à l'air nikel.

Avant de merger, ce serait bien que tu puisses tester quand même, donc je t'invites à comprendre ce qui ne va pas dans ton install. Et dès que tu nous donnes ton feu vert sur le fait que ton code marche comme ce à quoi tu t'attends je mergerais.

Reply to this email directly or view it on GitHubhttps://github.com/payutc/server/pull/328#issuecomment-37727931 .

roddehugo commented 10 years ago

Ca te dérange d'accepter la PR sans que je réinstalle tout :)

J'ai pas le temps et c'est Antoine qui prend le relai. Ce fut un plaisir de coder avec vous !

Version Slim : "2.4.0" Version PHP : "5.4.24"

Bonne journée

Hugo Rodde, Élève Ingénieur à l'Université de Technologie de Compiègne. Branche Génie Informatique 06 27 22 93 47

Le 16 mars 2014 10:36, Thomas Recouvreux notifications@github.com a écrit :

Weirdos Le 15 mars 2014 16:02, "Matthieu Guffroy" notifications@github.com a écrit :

Bon j'ai testé en faisant un loginApp avant, en donnant les droits à l'appli concerné. Et pour ce qui est de la méthode getProductsByCategories j'ai obtenu un retour avec les catégories et leurs produits. Pour ainsi dire ça à l'air nikel.

Avant de merger, ce serait bien que tu puisses tester quand même, donc je t'invites à comprendre ce qui ne va pas dans ton install. Et dès que tu nous donnes ton feu vert sur le fait que ton code marche comme ce à quoi tu t'attends je mergerais.

Reply to this email directly or view it on GitHub< https://github.com/payutc/server/pull/328#issuecomment-37727931> .

— Reply to this email directly or view it on GitHubhttps://github.com/payutc/server/pull/328#issuecomment-37752898 .

trecouvr commented 10 years ago

dans composer.json c'est slim 2.3., mais bon apres avoir installé slim 2.4. les tests passent donc c'est peut etre pas le probleme.

trecouvr commented 10 years ago

yop, testé en local avec slim 2.3 et 2.4, ca marche, je valide le PR. C'est regretable qu'il n'y ai aucun test cependant...