developer.jelix.org is not used any more and exists only for history. Post new tickets on the Github account.
developer.jelix.org n'est plus utilisée, et existe uniquement pour son historique. Postez les nouveaux tickets sur le compte github.

Opened 12 years ago

Closed 12 years ago

#561 closed bug (fixed)

Plugin history : gestion de la taille de l'historique

Reported by: tbosviel Owned by: Lipki
Priority: low Milestone: Jelix 1.1 beta 1
Component: jelix:plugins Version: trunk
Severity: minor Keywords:
Cc: Blocked By:
Blocking: Documentation needed: no
Hosting Provider: Php version:

Description

Lorsque la taille maximum de l'historique est atteinte, le plugin n'ajoute plus de pages. Il serait plus judicieux de supprimmer la page la plus ancienne et ajouter la nouvelle (FIFO).

Attachments (6)

history.diff (3.6 KB) - added by tbosviel 12 years ago.
history.2.diff (4.6 KB) - added by Lipki 12 years ago.
jelix-trunk-#561.patch (4.1 KB) - added by bibo 12 years ago.
patch to make history behave like a FIFO
jelix-trunk-#561.2.patch (4.1 KB) - added by bibo 12 years ago.
version 2
jelix-trunk-#561.3.patch (5.0 KB) - added by bibo 12 years ago.
third patch
jelix-trunk-#561.4.patch (5.1 KB) - added by bibo 12 years ago.
add a check on session variable

Download all attachments as: .zip

Change History (26)

Changed 12 years ago by tbosviel

comment:1 Changed 12 years ago by Lipki

C'est moi qui bug, ou on peut pas appliquer ton patch ... obliger de la faire a la main ...

comment:2 Changed 12 years ago by Lipki

  • Priority changed from normal to lowest
  • review changed from review? to review-
  • Severity changed from normal to minor
  • Type changed from enhancement to bug

Tu a bien corriger le bug dont tu parle, mais tu en a créer un nouveau.

Dit moi si tu veut corriger ou si je m'en occupe.
(Si tu veut corriger accepte le ticket (en zone action))
Et dit moi si tu ne trouve pas se qui bug.

Sinon je n'ai pas réussi a appliquer ton patch directement, je pense qu'il était mal localisé.
Et pense a remplacer les tabulations par 4 espaces.

comment:3 Changed 12 years ago by Lipki

PS: je suppose que tu voulais dire FEFO

comment:4 follow-up: Changed 12 years ago by tbosviel

  • Owner set to tbosviel
  • Status changed from new to assigned

Dit moi ce qui bug et je m'en occupe.

comment:5 in reply to: ↑ 4 Changed 12 years ago by Lipki

Alors si l'option single est activée, seul la toutes première instance de la page est supprimer. (fait des testes tu verra)

Parfois il persiste des doubles même si l'option double est a false.

comment:6 Changed 12 years ago by tbosviel

  • Owner changed from tbosviel to Lipki
  • Status changed from assigned to new

Bon, je n'ai pas réussi à reproduire les bugs dont tu parles. Je te laisse les corriger.

comment:7 Changed 12 years ago by Lipki

Merci pour ce retour, il ma permis de trouver une erreur de conception importante, en plus du bug signaler.

comment:8 Changed 12 years ago by Lipki

  • review changed from review- to review?

Changed 12 years ago by Lipki

comment:9 Changed 12 years ago by tbosviel

  • review changed from review? to review-

Le plugin fonctionne bien quand l'affichage de l'historique se fait dans le template enregistré dans la variable bodyTpl de la réponse.

En revanche quand l'affichage se fait dans un template généré dans l'action avec la méthode fetch() (ce qui arrive souvent quand on utilise des réponses communes personnalisées) la méthode beforeOutput() n'a pas encore été exécutée et l'historique est incomplet.

comment:10 Changed 12 years ago by Lipki

zut flute

Changed 12 years ago by bibo

patch to make history behave like a FIFO

comment:11 Changed 12 years ago by bibo

  • Priority changed from lowest to low
  • review changed from review- to review+

the latest patch corrects this ticket. Now history behaves like a FIFO.

ie when your history length becomes greater than maxsize parameter, its latest entry is deleted. and so on.

Otherwise, I don't know what Lipki thinks when he talk about misconception.

comment:12 Changed 12 years ago by laurentj

  • review changed from review+ to review?

Changed 12 years ago by bibo

version 2

comment:13 Changed 12 years ago by bibo

a new patch to correct a nit on the previous one.

comment:14 Changed 12 years ago by laurentj

  • Milestone set to Jelix 1.1 beta 1
  • review changed from review? to review+

It's ok for me.

Changed 12 years ago by bibo

third patch

comment:15 Changed 12 years ago by bibo

  • review changed from review+ to review?

During tests, I found another bug. This one resides in the test that prevent reload in the browser to bloat your history. It should not compare a 'page' to another but only 'action' and 'params' attribute as one could have changed 'title' or 'label' attributes...

The latest patch adds a new method isLast($action,$params=NULL) that gives a correct behaviour.

This new method is also a great helper. Let me explain. It is a pain to define 'label' and 'title' in every action particularly if you intend to reuse response title. It would be natural to define those attributes in a common response (think myhtmlresponse). The only drawback was that I could not know if an action was declared to step outside history ('history.add'=>false). Now you can see that isLast method serves also this purpose.

I'm not sure of isLast name but I can find a better one at the moment.

Asking review again.

Changed 12 years ago by bibo

add a check on session variable

comment:16 Changed 12 years ago by bballizlife

Ok so here we had several review+ without any commit. Never mind as it seems bugs were discovered or improvements made, but we need to going throw the process to the end.

At last, a patch was added (jelix-trunk-#561.4.patch). I guess it needs a review. So please set the Review property to review? so as your patch will be reviewed. Otherwise the patch could wait for a long time without being reviewed ;)

comment:17 Changed 12 years ago by bballizlife

Oops there already was the review? Sorry ;)

comment:18 Changed 12 years ago by bballizlife

So here i did not manage to test the patch, i had errors importing it. Hopefully, bibo gave me its history.coord.php so i could test the plugin. Now it works well as a FIFO stack and i do not see any other problems.

If someone can test the patch and confirm it will be great. Else, i don't know, you can trust me and allow bibo to commit its patch i guess ;)

Conclusion : ok for me.

comment:19 Changed 12 years ago by bballizlife

  • review changed from review? to review+

comment:20 Changed 12 years ago by bibo

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.