Y'a pas comme un truc qui vous fait hurler là ?

Le 11 novembre 2008 à 20h18 | Publié sous | 9 commentaires

Je n’ai pas vraiment le temps d’écrire en ce moment, malgré une trentaine d’articles en cours de rédaction, au point d’avoir fait sauter la sacro-sainte revue de presse de vendredi dernier.
Je tenais tout de même à vous faire partager ce bout de code digne du Daily WTF sur lequel je suis tombé tout à l’heure en travaillant sur la prochaine release de Typo, l’outil de publication en Ruby on Rails qui propulse ce site. Il se trouve visiblement là depuis plusieurs années, et je me demande si ce n’est pas à lui que mon blogware favori doit sa réputation de bloatware, malgré des optimisations déjà apportées par ailleurs.

Lecteur, ton oeil de lynx verra-t-il ce qui m’a fait hurler ?

Dans le contrôleur

 def index
   @articles = Article.find_all_by_date(*params.values_at(:year, :month, :day))
   @page_title = index_title
   @description = index_description
   @keywords = (this_blog.meta_keywords.empty?) ? "" : this_blog.meta_keywords
   
   respond_to do |format|
     format.html { render_paginated_index }
     format.atom do
       render :partial => 'articles/atom_feed', :object => @articles[0,this_blog.limit_rss_display]
     end
     format.rss do
       auto_discovery_feed(:only_path => false)
       render :partial => 'articles/rss20_feed', :object => @articles[0,this_blog.limit_rss_display]
     end
   end
 end

def render_paginated_index(on_empty = _("No posts found..."))
   return error(on_empty, :status => 200) if @articles.empty?

   @pages = Paginator.new self, @articles.size, this_blog.limit_article_display, params[:page]
   start = @pages.current.offset
   stop  = (@pages.current.next.offset - 1) rescue @articles.size
   # Why won't this work? @articles.slice!(start..stop)
   @articles = @articles.slice(start..stop)
   render :action => 'index'
 end

Dans le modèle

 # Find all articles on a certain date
 def self.find_all_by_date(year, month = nil, day = nil)
   if !year.blank?
     find_published(:all,
                    :conditions => { :published_at =>
                      time_delta(year,month,day) })
   else
     find_published(:all)
   end
 end

Et non, cette fois, il n’y a rien à gagner.

  1. Bruno Michel about 5 hours plus tard :

    C’est un peu bourrin de charger tous les articles pour n’en afficher que quelques uns, et oui, cela ne doit pas aider pour les perfs.

  2. Bruno Michel about 5 hours plus tard :

    Il semble y avoir un bug sur le formulaire pour laisser un commentaire. Il envoie sur l’URL http://t37.netya-pas-comme-un-truc-qui-vous-fait-hurler-li/comments (il manque un / entre t37.net et le slug du billet).

  3. NiKo about 13 hours plus tard :

    Haha, fortune.

  4. Frédéric de Villamil about 14 hours plus tard :

    Bruno : effectivement, j’avais un soucis depuis la migration de mon thème vers HEAD. Ça pourrait expliquer pourquoi je n’ai pas eu de commentaires depuis une semaine en fait

  5. Harry Seldon 3 days plus tard :

    Heu tout cela ne me parait pas très clair. En revanche je suis interessé par connaître le code à changer si c’est simple et que ça peut être fait sur la v5.1.2. Merci

  6. Frédéric de Villamil 3 days plus tard :

    @Harry Seldon : en gros, aujourd’hui, quand tu veux afficher n’importe quelle page de ton blog hors billet seul, Typo charge toute la table articles en mémoire (plus la table feedbacks, la table categories et la table tags, relations obligent). Puis, il prend les lignes du tableau qui l’intéressent (généralement une dizaine) et affiche.

    Pour les tags et les catégories, c’est un tout petit peu moins bourrin puisqu’il ne fait qu’avec les articles liés à l’article ou au tag en cours.

    Cette aberration est due à l’ancien système de pagination de Rails qui calculait la pagination à partir d’un tableau contenant l’intégralité des billets.

    Et les réponses à tes questions sont :

    Le code à changer se trouve un peu partout dans la version de développement, et pour cette partie, dans le commit http://github.com/fdv/typo/commit/30f089a940be74b5303af787e119c24297fb0103

    Si c’est simple : non. Notamment parce que ça nécessite de : - changer le système de pagination (OK simple) - partout dans l’admin (un peu moins) - partout coté front end (amuse toi) - dans le thème - ce qui implique notamment de modifier assez lourdement le modèle article.rb un chouilla monolithique afin de l’adapter à notre ami will_paginate - modifier un certain nombre de tests

    Est-ce que ça peut être fait sur la 5.1.2 : oui, mais honnêtement, vu la lourdeur des modifications que ça entraîne, ce serait vraiment très con. Autant passer sous trunk.

  7. Harry Seldon 3 days plus tard :

    OK Merci pour la réponse.

    Je vais essayer de passer sous trunk. Mais bon ça demande un peu de sport vu qu’il faut passer sous git (cf http://harryseldon.thinkosphere.com/2008/11/08/grand-gardening-with-gi) et qu’il faut passer sous Rails 2.2. (Même pas sûr que mon hébergeur l’ait, du coup il faudra que je customise mon install).
    En tout cas, je suis en train. Au passage t’as du recevoir un patchoune (très petit patch) via github.

    Au passage quelle est la raison fondamentale pour laquelle Rails 2.1 n’est pas supporté par trunk ?

    Y-a-t-il des points particuliers auquel il faut attention pour la migration de la 5.1.2 à trunk ?

  8. Harry Seldon 3 days plus tard :

    J’ai mal copié/collé mon lien*. Ca me fait penser qu’un de mes utilisateurs m’a aussi fait la remarque que ce serait cool si les utilisateurs pouvaient éditer leurs commentaires a posteriori.
    Cependant j’imagine bien que ce ne doit pas être si simple que ça (il faut mettre en place un système de login etc.)

  9. Frédéric de Villamil 3 days plus tard :

    Alors, pour rails 2.2, il est inclut dans les submodules. Donc un coup de git submodule init puis git submodule update dans ton Typo et tu l’as.

    J’ai bien reçu le patch, et tu y as rajouté une typo : en anglais, tu ne mets pas d’espace insécable entre la fin de la phrase et le ? contrairement au français ou tous les signes de ponctuation doubles (?, ;, :, ! …) sont précédés d’un espace insécable.

    La raison fondamentale au non support de Rails 2.1, c’est que j’ai décidé de ne pas faire de release de Typo pour cette version, de manière totalement arbitraire, parce que je n’avais pas le temps de faire la migration.

    Et pour les points importants, je dirais backup complet de ta base de données, et rake db:migrate RAILS_ENV=production avant de redémarrer ton Typo

    Et pour les inscriptions, on peut, mais elles ne sont pas activées chez moi.

Merci de vous exprimer dans un français correct. Les commentaires déplacés, injurieux et le spam seront supprimés.

Les trackbacks sont fermés pour cause de spam.