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

[beta] La liste des sujets suivis n'est pas cohérente avec la réalité #2873

Closed
firm1 opened this issue Jul 1, 2015 · 42 comments
Closed
Labels
Bloquant Ticket qui doit être traité avant la prochaine mise à jour C-Back Concerne le back-end Django S-Régression Corrige un problème sur un composant qui fonctionnait auparavant
Milestone

Comments

@firm1
Copy link
Contributor

firm1 commented Jul 1, 2015

Url incriminée : https://beta.zestedesavoir.com/forums/notifications/

Comme en témoigne le screen ci-dessous, lorsque j'ai un sujet suivis que je n'ai pas encore lu, dans la sidebar le gras est visible et montre que le topic n'est pas lu, mais dans la liste qui s'affiche on a pas de graisse pour différencier les topic lu ou non.

graisser

Vu que ça marche bien en prod, j'en déduis qu'on tiens une regression.

@firm1 firm1 added C-Front Concerne l'interface du site Facile Bon ticket pour débuter pour rejoindre le développement ! S-Régression Corrige un problème sur un composant qui fonctionnait auparavant C-Back Concerne le back-end Django labels Jul 1, 2015
@SpaceFox
Copy link
Contributor

SpaceFox commented Jul 1, 2015

Voilà :

@firm1
Copy link
Contributor Author

firm1 commented Jul 1, 2015

Haha, you made my day :P

@Eskimon
Copy link
Contributor

Eskimon commented Jul 1, 2015

Du coup tu as bien fait de tagguer en "ré-graisse-ion" ;)

@firm1
Copy link
Contributor Author

firm1 commented Jul 1, 2015

:P

Et sinon, le bug est aussi visible dans la liste des sujets d'un topic.

@Situphen
Copy link
Member

Situphen commented Jul 1, 2015

Ça m'a l'air lié à #2785 ! Ping @artragis ;)

@artragis
Copy link
Member

artragis commented Jul 2, 2015

nope, je ne touche pas à cette page là. C'est plutôt le refacto des forums qui a possiblement été cassé.

@Situphen
Copy link
Member

Situphen commented Jul 2, 2015

@artragis
Copy link
Member

artragis commented Jul 2, 2015

c'est bon, j'ai vu pourquoi !. Je fais une PR ce midi.

2015-07-02 11:46 GMT+02:00 Situphen notifications@github.com:

Si, tu touches au topic_row.part.html
https://github.com/zestedesavoir/zds-site/commits/dev/templates/forum/includes/topic_row.part.html
et plus précisément à la condition if qui met en "unread" ou pas le topic
https://github.com/zestedesavoir/zds-site/blame/dev/templates/forum/includes/topic_row.part.html#L8
!


Reply to this email directly or view it on GitHub
#2873 (comment)
.

@artragis
Copy link
Member

artragis commented Jul 2, 2015

PR faite et j'ai même fait ça en mode TDD (i.e le teste fail, je développe, le test passe).

@pierre-24
Copy link
Member

Et PR mergée, merci @artragis :)

@pierre-24 pierre-24 added this to the Version 15.6 milestone Jul 4, 2015
@pierre-24 pierre-24 removed the C-Front Concerne l'interface du site label Jul 4, 2015
@artragis
Copy link
Member

artragis commented Jul 4, 2015

mais de rien pierre :p

Le 04/07/2015 10:47, Pierre Beaujean a écrit :

Closed #2873 #2873.


Reply to this email directly or view it on GitHub
#2873 (comment).

@SpaceFox
Copy link
Contributor

SpaceFox commented Jul 4, 2015

C'est absolument pas corrigé, et même plus large que ça : sur les forums, je n'ai aucun sujet en gras.

C'est le genre de gag qui va mériter un hotfix...

PS : Je dégage le tag "facile" puisque de toute évidence ce n'est pas le cas.

@SpaceFox SpaceFox reopened this Jul 4, 2015
@SpaceFox SpaceFox added Bloquant Ticket qui doit être traité avant la prochaine mise à jour and removed Facile Bon ticket pour débuter pour rejoindre le développement ! labels Jul 4, 2015
@SpaceFox
Copy link
Contributor

SpaceFox commented Jul 4, 2015

Apparemment le problème n'a été corrigé que dans la page des notifications alors qu'il était présent partout dans les forums.

@artragis
Copy link
Member

artragis commented Jul 5, 2015

J'avais prévenu que c'était possible. J'avais demandé à ce que ça soit
vérifié, ça ne l'a pas été. Le problème c'est que là, je ne comprends
pas pourquoi ça fait ça puisque le test unitaire fonctionne et donne des
résultats cohérents.

Le 05/07/2015 01:14, SpaceFox a écrit :

Apparemment le problème n'a été corrigé que dans la page des
notifications alors qu'il était présent partout dans les forums.


Reply to this email directly or view it on GitHub
#2873 (comment).

@pierre-24
Copy link
Member

Ce à quoi je répond "qu'est ce que what the fuck", quand j'ai fait la QA ça marchait nickel (même la liste des forums). Là, on dirait que la PR n'a eu absolument aucun effet, puisque rien ne s'est remis en gras.

D'AILLEURS !

Lorsque je checkout (oui, c'est aussi mal que fix) la branche d'artragis correspondant à la PR, j'ai bel et bien du gras là ou je devrait.

Liste des notifs:

screenshot from 2015-07-05 08 45 13

Liste des forums:

screenshot from 2015-07-05 08 46 44

Alors désolé, cette PR fonctionnait TRÈS BIEN, le problème se situe ailleurs. Non mais.

@artragis
Copy link
Member

artragis commented Jul 5, 2015

Ouf ! car là je comprenais pas.

Le 05/07/2015 08:48, Pierre Beaujean a écrit :

Ce à quoi je répond "qu'est ce que /what the fuck/", quand j'ai fait
la QA ça marchait nickel (même la liste des forums). Là, on dirait que
la PR n'a eu absolument aucun effet, puisque rien ne s'est remis en gras.

D'AILLEURS !

Lorsque je /checkout/ (oui, c'est aussi mal que /fix/) la branche
d'artragis correspondant à la PR, j'ai bel et bien du gras là ou je
devrait.

Liste des notifs:

screenshot from 2015-07-05 08 45 13
https://cloud.githubusercontent.com/assets/7889675/8510603/4d22c722-22f2-11e5-8f8a-992c754582ca.png

Liste des forums:

screenshot from 2015-07-05 08 46 44
https://cloud.githubusercontent.com/assets/7889675/8510604/6998fe26-22f2-11e5-9efd-6ffefa7690f0.png

Alors désolé, cette PR fonctionnait TRÈS BIEN, le problème se situe
ailleurs. Non mais.


Reply to this email directly or view it on GitHub
#2873 (comment).

@pierre-24
Copy link
Member

Du coup, à priori, le hotfix revient à prendre la branche d'atragis et à la ré-appliquer sur prod, c'est probablement aussi simple que ça.

@artragis
Copy link
Member

artragis commented Jul 5, 2015

avant, il faudrait détecter la différence entre ma branche et la prod.

Le 05/07/2015 08:54, Pierre Beaujean a écrit :

Du coup, à priori, le hotfix revient à prendre la branche d'atragis et
à la ré-appliquer sur /prod/, c'est probablement aussi simple que ça.


Reply to this email directly or view it on GitHub
#2873 (comment).

@pierre-24
Copy link
Member

Je trouve pas, mais peut-être que c'est plus pervers que ça. En effet, quelque sois la branche (donc prod aussi), j'ai bien du gras quand

  • Je suis sensé avoir une notif sur le sujet en question :

screenshot from 2015-07-05 09 38 19

  • Il y a un nouveau sujet que je n'ai jamais lu :

screenshot from 2015-07-05 09 38 51

Donc à mon avis, on se tape quelque chose de plus pervers que ça (mais je sais pas quoi). Quelqu'un peu pas aller générer une notif sur le forum ?

@artragis
Copy link
Member

artragis commented Jul 5, 2015

j'ai la vague impressionq ue notre problème de https empêche à la
condition is_authenticated d'être validée.

Le 05/07/2015 09:39, Pierre Beaujean a écrit :

Je trouve pas, mais peut-être que c'est plus pervers que ça. En effet,
quelque sois la branche (donc /prod/ aussi), j'ai bien du gras quand

  • Je suis sensé avoir une notif sur le sujet en question :

screenshot from 2015-07-05 09 38 19
https://cloud.githubusercontent.com/assets/7889675/8510758/97cab2a6-22f9-11e5-8e1e-ba416443d47d.png

  • Il y a un nouveau sujet que je n'ai jamais lu :

screenshot from 2015-07-05 09 38 51
https://cloud.githubusercontent.com/assets/7889675/8510759/ac184412-22f9-11e5-93de-a9d4ddb835a0.png

Donc à mon avis, on se tape quelque chose de plus pervers que ça (mais
je sais pas quoi). Quelqu'un peu pas aller générer une notif sur le
forum ?


Reply to this email directly or view it on GitHub
#2873 (comment).

@pierre-24
Copy link
Member

Bien essayé, mais vu qu'on a un souci de certif', je navigue en HTTP pour le moment, et j'ai bien pas de gras sur le site. À moins que ça ne soit quand même lié.

@artragis
Copy link
Member

artragis commented Jul 5, 2015

tu arrives à être en http ET authentifié?

Le 05/07/2015 09:42, Pierre Beaujean a écrit :

Bien essayé, mais vu qu'on a un souci de certif', je navigue en HTTP
pour le moment, et j'ai bien pas de gras sur le site. À moins que ça
ne soit quand même lié.


Reply to this email directly or view it on GitHub
#2873 (comment).

@pierre-24
Copy link
Member

Comme tu peux le voir: voir message après.

screenshot from 2015-07-05 09 43 36

(tu peux aussi constater que ta notif' ne m'as pas généré de gras)

@pierre-24
Copy link
Member

EDIT: mauvais screen :p

screenshot from 2015-07-05 09 44 53

@artragis
Copy link
Member

artragis commented Jul 5, 2015

Par contre dans la page "toutes les notifs" on est ok?

Le 05/07/2015 09:44, Pierre Beaujean a écrit :

Comme tu peux le voir:

screenshot from 2015-07-05 09 43 36
https://cloud.githubusercontent.com/assets/7889675/8510772/536689e0-22fa-11e5-9498-9337c9c03a3d.png

(tu peux aussi constater que ta notif' ne m'as pas généré de gras)


Reply to this email directly or view it on GitHub
#2873 (comment).

@pierre-24
Copy link
Member

Ça attendra la prochaine notif', j'ai cliqué :p

@pierre-24
Copy link
Member

Ça le fait effectivement sur la liste des notifs':

screenshot from 2015-07-05 10 16 08

(donc reste à comprendre pourquoi ça le fait en local et pas ici)

@pierre-24
Copy link
Member

Aller, l'histoire de bien en rajouter: la bêta est iso-prod, son certif' HTTPS est valide et correct et ça bugge quand même. Par acquis de conscience, j'ai essayé de regénérer le front, et c'est pas ça non plus. Fait ch***

@artragis
Copy link
Member

artragis commented Jul 5, 2015

si tu as les droits sur la beta, peux-tu temporairement mettre le
"debug" à True histoire qu'on ait un log des requêtes.

Le 05/07/2015 11:03, Pierre Beaujean a écrit :

Aller, l'histoire de bien en rajouter: la bêta est /iso/-prod, son
certif' HTTPS est valide et correct et ça bugge quand même. Par acquis
de conscience, j'ai essayé de regénérer le /front/, et c'est pas ça
non plus. Fait ch***


Reply to this email directly or view it on GitHub
#2873 (comment).

@pierre-24
Copy link
Member

Je n'ai pas ce plaisir ;)

@sandhose
Copy link
Contributor

sandhose commented Jul 5, 2015

si tu as les droits sur la beta, peux-tu temporairement mettre le
"debug" à True histoire qu'on ait un log des requêtes.

Je peux le faire ; aussi, c'est sûr que c'est pas un problèmes front, puisque les éléments ont simplement pas la classe .unread

@artragis
Copy link
Member

artragis commented Jul 5, 2015

oui, c'est pour ça que je veux voir les requêtes.

Le 05/07/2015 11:18, Sandhose a écrit :

si tu as les droits sur la beta, peux-tu temporairement mettre le
"debug" à True histoire qu'on ait un log des requêtes.

Je peux le faire ; aussi, c'est sûr que c'est pas un problèmes front,
puisque les éléments ont simplement pas la classe |.unread|


Reply to this email directly or view it on GitHub
#2873 (comment).

@artragis
Copy link
Member

artragis commented Jul 5, 2015

Merci, là j'ai juste un problème : ce salop n'envoie pas la requête. il
fait juste ça :

/opt/zdsenv/bin/gunicorn  in<module>(9)
   load_entry_point('gunicorn==19.3.0', 'console_scripts', 'gunicorn')()
/opt/zdsenv/local/lib/python2.7/site-packages/gunicorn/app/wsgiapp.py  inrun(74)
   WSGIApplication("%(prog)s [OPTIONS] [APP_MODULE]").run()
/opt/zdsenv/local/lib/python2.7/site-packages/gunicorn/app/base.py  inrun(189)
   super(Application, self).run()
/opt/zdsenv/local/lib/python2.7/site-packages/gunicorn/app/base.py  inrun(72)
   Arbiter(self).run()
/opt/zdsenv/local/lib/python2.7/site-packages/gunicorn/arbiter.py  inrun(174)
   self.manage_workers()
/opt/zdsenv/local/lib/python2.7/site-packages/gunicorn/arbiter.py  inmanage_workers(477)
   self.spawn_workers()
/opt/zdsenv/local/lib/python2.7/site-packages/gunicorn/arbiter.py  inspawn_workers(540)
   self.spawn_worker()
/opt/zdsenv/local/lib/python2.7/site-packages/gunicorn/arbiter.py  inspawn_worker(507)
   worker.init_process()
/opt/zdsenv/local/lib/python2.7/site-packages/gunicorn/workers/base.py  ininit_process(124)
   self.run()
/opt/zdsenv/local/lib/python2.7/site-packages/gunicorn/workers/sync.py  inrun(119)
   self.run_for_one(timeout)
/opt/zdsenv/local/lib/python2.7/site-packages/gunicorn/workers/sync.py  inrun_for_one(66)
   self.accept(listener)
/opt/zdsenv/local/lib/python2.7/site-packages/gunicorn/workers/sync.py  inaccept(30)
   self.handle(listener, client, addr)
/opt/zdsenv/local/lib/python2.7/site-packages/gunicorn/workers/sync.py  inhandle(130)
   self.handle_request(listener, req, client, addr)
/opt/zdsenv/local/lib/python2.7/site-packages/gunicorn/workers/sync.py  inhandle_request(171)
   respiter = self.wsgi(environ, resp.start_response)
/opt/zdsenv/ZesteDeSavoir/zds/forum/views.py  inget(73)
   return super(ForumTopicsListView, self).get(request, *args, **kwargs)
/opt/zdsenv/ZesteDeSavoir/zds/forum/views.py  inget_context_data(82)
   'topic_read': TopicRead.objects.list_read_topic_pk(self.request.user, context['topics'])
/opt/zdsenv/ZesteDeSavoir/zds/forum/managers.py  inlist_read_topic_pk(110)
   return self.topic_read_by_user(user, topic_sub_list).values_list('topic__pk', flat=True)
/opt/zdsenv/ZesteDeSavoir/zds/forum/managers.py  intopic_read_by_user(102)
   base_query_set = self.filter(user__pk=user.pk)

ce qui génère la requête

*SELECT*   `auth_user`.`id`,
  `auth_user`.`password`, `auth_user`.`last_login`,
`auth_user`.`is_superuser`, `auth_user`.`username`,
`auth_user`.`first_name`, `auth_user`.`last_name`, `auth_user`.`email`,
`auth_user`.`is_staff`, `auth_user`.`is_active`,
`auth_user`.`date_joined`  <https://beta.zestedesavoir.com/forums/communaute/dev-zone/#>  *FROM*  `auth_user`*WHERE*  `auth_user`.`id` = 82*LIMIT*  21

c'est vraiment mystique là.

Le 05/07/2015 11:18, Sandhose a écrit :

si tu as les droits sur la beta, peux-tu temporairement mettre le
"debug" à True histoire qu'on ait un log des requêtes.

Je peux le faire ; aussi, c'est sûr que c'est pas un problèmes front,
puisque les éléments ont simplement pas la classe |.unread|


Reply to this email directly or view it on GitHub
#2873 (comment).

@pierre-24
Copy link
Member

Attend, j'essaye de comprendre, c'est quoi le problème, là ?

@pierre-24
Copy link
Member

Il passe pas non plus par {% if user.is_authenticated and topic.pk in topic_read %} de topic_row.part.html dans lequel il est censé passer pour rajouter le unread qui va bien.

Est ce qu'on serait pas sur ce genre de bug un peu ennuyeux mysql/sqlite ?

@artragis
Copy link
Member

artragis commented Jul 5, 2015

il ne lance pas la requête qui sélectionne tous les topics lus.

ce qui d'ailleurs me casse les pieds puisque du coup théoriquement
"topic_read" devrait être vide et donc TOUS les topics devraient être
marqués "non lus" et non "lus".

Mystique.

Le 05/07/2015 11:51, Pierre Beaujean a écrit :

Attend, j'essaye de comprendre, c'est quoi le problème, là ?


Reply to this email directly or view it on GitHub
#2873 (comment).

@artragis
Copy link
Member

artragis commented Jul 5, 2015

Est ce qu'on serait pas sur ce genre de bug un peu ennuyeux mysql/sqlite ?

c'est pas impossible.

Le 05/07/2015 11:57, Pierre Beaujean a écrit :

Est ce qu'on serait pas sur ce genre de bug un peu ennuyeux mysql/sqlite ?

@pierre-24
Copy link
Member

Grâce à @artragis, on a pu confirmer qu'il s'agissait bien d'un de ces bugs retords qui apparaissent quand on commence à employer mysql alors que ça fonctionne très bien en local avec sqlite.

@SpaceFox
Copy link
Contributor

SpaceFox commented Jul 5, 2015

_<

@pierre-24
Copy link
Member

Donc je fait des test avec MySQL. Et j'ai trouvé un truc intéressant/débile.

La fonction topic_read_by_user() prend en paramètre une liste de sujet, topic_sub_list. Si topic_sub_list est de type list, tout va très bien (c'est le cas pour la liste des notifs, parc'e que ça sort du paginateur). Si topic_sub_list est un QuerySet, alors c'est le bazar, et ça fini en

1235 -- This version of MySQL doesn't yet support 'LIMIT & IN/ALL/ANY/SOME subquery

(je parie que le log Django en est rempli, du coup).

Le fix est complètement idiot: topic_sub_list=[i for i in quesryset]. Et il semblerai que les performances soient pas impactées/améliorées. brace yourself, fix is coming !

@artragis
Copy link
Member

artragis commented Jul 5, 2015

ou bien simplement ".all()"

Le 05/07/2015 14:50, Pierre Beaujean a écrit :

Donc je fait des test avec MySQL. Et j'ai trouvé un truc
intéressant/débile.

La fonction |topic_read_by_user()|
https://github.com/zestedesavoir/zds-site/blob/dev/zds/forum/managers.py#L100
prend en paramètre une liste de sujet, |topic_sub_list|. Si
|topic_sub_list| est de type |list|, tout va très bien (c'est le cas
pour la liste des notifs, parc'e que ça sort du paginateur). Si
|topic_sub_list| est un |QuerySet|, alors c'est le bazar, et ça fini en

|1235 -- This version of MySQL doesn't yet support 'LIMIT & IN/ALL/ANY/SOME subquery
|

(je parie que le log Django en est rempli, du coup).

Le fix est complètement idiot: |topic_sub_list=[i for i in
quesryset]|. Et il semblerai que les performances soient pas
impactées/améliorées. /brace yourself, fix is coming/ !


Reply to this email directly or view it on GitHub
#2873 (comment).

artragis added a commit to artragis/zds-site that referenced this issue Jul 5, 2015
Mysql ne permet pas d'utiliser un queryset comme si c'était une liste, j'ai donc fait des "all" juste avant.
@artragis
Copy link
Member

artragis commented Jul 5, 2015

j'ai proposé un fix

Le 05/07/2015 14:50, Pierre Beaujean a écrit :

Donc je fait des test avec MySQL. Et j'ai trouvé un truc
intéressant/débile.

La fonction |topic_read_by_user()|
https://github.com/zestedesavoir/zds-site/blob/dev/zds/forum/managers.py#L100
prend en paramètre une liste de sujet, |topic_sub_list|. Si
|topic_sub_list| est de type |list|, tout va très bien (c'est le cas
pour la liste des notifs, parc'e que ça sort du paginateur). Si
|topic_sub_list| est un |QuerySet|, alors c'est le bazar, et ça fini en

|1235 -- This version of MySQL doesn't yet support 'LIMIT & IN/ALL/ANY/SOME subquery
|

(je parie que le log Django en est rempli, du coup).

Le fix est complètement idiot: |topic_sub_list=[i for i in
quesryset]|. Et il semblerai que les performances soient pas
impactées/améliorées. /brace yourself, fix is coming/ !


Reply to this email directly or view it on GitHub
#2873 (comment).

SpaceFox added a commit that referenced this issue Jul 13, 2015
[hotfix v15.6] fix #2873 avec mysql
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bloquant Ticket qui doit être traité avant la prochaine mise à jour C-Back Concerne le back-end Django S-Régression Corrige un problème sur un composant qui fonctionnait auparavant
Projects
None yet
Development

No branches or pull requests

7 participants