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

Poste un message dans un message privé avec des messages depuis l'API #3443

Merged
merged 2 commits into from
Mar 27, 2016
Merged

Poste un message dans un message privé avec des messages depuis l'API #3443

merged 2 commits into from
Mar 27, 2016

Conversation

GerardPaligot
Copy link
Member

Q R
Correction de bugs ? Oui
Nouvelle Fonctionnalité ? Non
Tickets (issues) concernés #3442

QA :

  1. Fournissez-vous de tokens d'authentification et récupérez l'identifiant d'un message privé.
  2. Faites une requête depuis l'API pour poster un nouveau message dans ce message privé.
  3. Constatez que le message est bien posté.

@GerardPaligot GerardPaligot changed the title fix(api): Creates a post in a PM with messages. Poste un message dans un message privé avec des messages depuis l'API Mar 12, 2016
@gustavi gustavi added S-BUG Corrige un problème C-Back Concerne le back-end Django C-API Concerne une API du site labels Mar 13, 2016
@SpaceFox
Copy link
Contributor

On peut passer ça en hotfix

@artragis
Copy link
Member

je pense, en effet.

@GerardPaligot
Copy link
Member Author

Un autre hotfix va venir dans la journée concernant l'authentification OAuth2.

@SpaceFox
Copy link
Contributor

Du coup @GerardPaligot on en est où dans cette histoire de hotfix ?

@GerardPaligot
Copy link
Member Author

@SpaceFox J'attends patiemment une QA. :)

@artragis
Copy link
Member

Pour l'API la QA c'est chiant (en fait j'ai jamais réussi à faire un
processus d'auth complet perso) et je me dis qu'en plus les TU sont la
QA vu comment ils sont faits.

Le 20/03/2016 09:19, Gérard Paligot a écrit :

@SpaceFox https://github.com/SpaceFox J'attends patiemment une QA. :)


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#3443 (comment)

@gustavi
Copy link
Contributor

gustavi commented Mar 20, 2016

Je rejoins @artragis là dessus.

@GerardPaligot
Copy link
Member Author

Qu'est ce que vous trouvez compliqué avec l'authentification de l'API ?

@DevHugo
Copy link
Contributor

DevHugo commented Mar 20, 2016

Si ça peut attendre vendredi prochain, je pourrais faire la QA. Mais je peux rien faire avant. Désolé.

@gustavi
Copy link
Contributor

gustavi commented Mar 20, 2016

@GerardPaligot c'est assez complexe à faire. J'ajouterai que normalement chaque méthode de l'API devrait avoir un TU non ? Parce que en soit le comportement de l'API est automatique donc les TU devraient être suffisants.

@DevHugo
Copy link
Contributor

DevHugo commented Mar 20, 2016

@GerardPaligot

QA: Nope =( .

  • Je constate bien une 400 sur la branche dev
  • Si je passe bien sur la nouvelle branche, je peux effectivement poster un message dans une conversation ou je suis présent -> Ok
  • Toujours sur la nouvelle branche, je ne peux pas poster un message dans une conversation ou je n'est pas les droits -> Ok
  • Je peux poster un message dans une conversation ou je suis seul =( -> Pas Ok.

Pour reproduire:

Avec un compte admin, valider un tutoriel de user ou sanctionner user pour recevoir un message ou tu sera tout seul dans la conversation.
Tenter de poster un message avec le compte user.

screenshot_20160320_113802

@GerardPaligot
Copy link
Member Author

Je peux poster un message dans une conversation ou je suis seul =( -> Pas Ok.

Tu ne peux pas poster un message dans lequel tu es seul. :)

@DevHugo
Copy link
Contributor

DevHugo commented Mar 20, 2016

Justement c'est ça le souci: je suis seul dans la conversation, et depuis l'api, je peux encore ajouter des messages, ce qui devrait être impossible.

@GerardPaligot
Copy link
Member Author

Voilà pourquoi les QA sont importantes. ^^ cc @artragis @gustavi

@gustavi
Copy link
Contributor

gustavi commented Mar 20, 2016

@GerardPaligot => TU !

@GerardPaligot
Copy link
Member Author

PR mise à jour.

@coveralls
Copy link

Coverage Status

Coverage decreased (-17.4%) to 70.546% when pulling 32915ba on GerardPaligot:fix_3442 into fd33c2b on zestedesavoir:prod.

@GerardPaligot
Copy link
Member Author

Quelqu'un sait pourquoi j'ai un échec dans les tests qui n'a rien à voir avec ma PR ?

@artragis
Copy link
Member

parce que ta PR n'est pas à jour. Le merge de la prod a rendu le TU de la génération de PDF foireux et tu t'es basé sur ça pour faire ta PR. Le TU a été corrigé sur dev.

@GerardPaligot
Copy link
Member Author

Bizarre. Ma PR était au vert avant pourtant et la branche prod n'a pas changé.

@artragis
Copy link
Member

c'est travis qui foire alors

@DevHugo
Copy link
Contributor

DevHugo commented Mar 20, 2016

Je te laisse régler ça avant de QA une dernière fois.

@GerardPaligot
Copy link
Member Author

Je peux rien faire malheureusement.

@DevHugo
Copy link
Contributor

DevHugo commented Mar 21, 2016

Je laisse la main pour la QA. Sinon, je le ferai vendredi.

@GerardPaligot
Copy link
Member Author

Cette PR m'ennuie un peu. Quelqu'un à une idée de savoir comment faire passer Travis ?

@artragis
Copy link
Member

A part faire la PR sur dev ou bien cherry picker le commit qui résoud le
truc, je sais pas.

2016-03-22 9:52 GMT+01:00 Gérard Paligot notifications@github.com:

Cette PR m'ennuie un peu. Quelqu'un à une idée de savoir comment faire
passer Travis ?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#3443 (comment)

@GerardPaligot
Copy link
Member Author

@artragis C'est quoi le commit concerné par le fix ?

@artragis
Copy link
Member

35c2f72

2016-03-22 9:57 GMT+01:00 Gérard Paligot notifications@github.com:

@artragis https://github.com/artragis C'est quoi le commit concerné par
le fix ?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#3443 (comment)

@GerardPaligot
Copy link
Member Author

Ecrire "plouf" règle le problème ? :|

Edit : Ok, expliqué par IRC. Nous ne générons plus des PDFs pertinents par rapport à nos tests.

@artragis
Copy link
Member

non, c'est FakePDFPublicator qui le règle

2016-03-22 10:04 GMT+01:00 Gérard Paligot notifications@github.com:

Ecrire "plouf" règle le problème ? :|


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#3443 (comment)

@GerardPaligot
Copy link
Member Author

@SpaceFox Ca te va si j'embarque le commit dans ma PR hotfix ?

@SpaceFox
Copy link
Contributor

Lui ? 35c2f72

@GerardPaligot
Copy link
Member Author

@SpaceFox Lui-même

@SpaceFox
Copy link
Contributor

OK.

@coveralls
Copy link

Coverage Status

Coverage decreased (-17.5%) to 70.538% when pulling 0fb9c02 on GerardPaligot:fix_3442 into fd33c2b on zestedesavoir:prod.

@GerardPaligot
Copy link
Member Author

Bien, PR ok du coup.

@DevHugo
Copy link
Contributor

DevHugo commented Mar 22, 2016

C'est urgent ou pas ?

@GerardPaligot
Copy link
Member Author

Je ne pense pas non. Un hotfix peut encore être appliqué quand une nouvelle release est en cours.

@DevHugo
Copy link
Contributor

DevHugo commented Mar 26, 2016

QA: Ok

Cas testés:

  • Si je passe bien sur la nouvelle branche, je peux effectivement poster un message dans une conversation ou je suis présent
  • Toujours sur la nouvelle branche, je ne peux pas poster un message dans une conversation ou je n'est pas les droits
  • Toujours sur la nouvelle branche, Je ne peux pas poster un message dans une conversation ou je suis seul

@GerardPaligot
Copy link
Member Author

Merci @DevHugo !

Ca peut se merger alors et une nouvelle version peut être déployée (cc @SpaceFox).

@gustavi gustavi merged commit 3177ac5 into zestedesavoir:prod Mar 27, 2016
@GerardPaligot GerardPaligot deleted the fix_3442 branch March 27, 2016 09:29
SpaceFox added a commit that referenced this pull request Mar 29, 2016
Poste un message dans un message privé avec des messages depuis l'API
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-API Concerne une API du site C-Back Concerne le back-end Django S-BUG Corrige un problème
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants