Ticket #561 (closed bug: fixed)

Opened 4 months ago

Last modified 2 months ago

Plugin history : gestion de la taille de l'historique

Reported by: tbosviel Assigned to: Lipki
Priority: low Milestone: Jelix 1.1 beta 1
Component: jelix:plugins Version: trunk
Severity: minor Keywords:
Cc: Php version:
Review: review+ Hosting Provider:
Documentation needed: 0 Blocking:

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

history.diff (3.6 kB) - added by tbosviel on 04/26/08 08:16:51.
history.2.diff (4.6 kB) - added by Lipki on 04/30/08 19:50:24.
jelix-trunk-#561.patch (4.1 kB) - added by bibo on 06/05/08 00:22:58.
patch to make history behave like a FIFO
jelix-trunk-#561.2.patch (4.1 kB) - added by bibo on 06/18/08 10:14:27.
version 2
jelix-trunk-#561.3.patch (5.0 kB) - added by bibo on 06/23/08 11:37:26.
third patch
jelix-trunk-#561.4.patch (5.1 kB) - added by bibo on 07/01/08 11:55:27.
add a check on session variable

Change History

04/26/08 08:16:51 changed by tbosviel

  • attachment history.diff added.

04/27/08 21:55:05 changed by Lipki

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

04/27/08 22:19:09 changed by Lipki

  • priority changed from normal to lowest.
  • review changed from review? to review-.
  • type changed from enhancement to bug.
  • severity changed from normal to minor.

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.

04/27/08 22:22:54 changed by Lipki

PS: je suppose que tu voulais dire FEFO

(follow-up: ↓ 5 ) 04/27/08 23:30:03 changed by tbosviel

  • owner set to tbosviel.
  • status changed from new to assigned.

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

(in reply to: ↑ 4 ) 04/29/08 11:45:19 changed 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.

04/29/08 15:32:18 changed 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.

04/29/08 17:20:07 changed by Lipki

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

04/30/08 19:49:37 changed by Lipki

  • review changed from review- to review?.

04/30/08 19:50:24 changed by Lipki

  • attachment history.2.diff added.

05/01/08 19:56:02 changed 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.

05/01/08 20:30:10 changed by Lipki

zut flute

06/05/08 00:22:58 changed by bibo

  • attachment jelix-trunk-#561.patch added.

patch to make history behave like a FIFO

06/05/08 00:31:49 changed 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.

06/18/08 00:58:59 changed by laurentj

  • review changed from review+ to review?.

06/18/08 10:14:27 changed by bibo

  • attachment jelix-trunk-#561.2.patch added.

version 2

06/18/08 10:16:20 changed by bibo

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

06/18/08 11:46:14 changed by laurentj

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

It's ok for me.

06/23/08 11:37:26 changed by bibo

  • attachment jelix-trunk-#561.3.patch added.

third patch

06/23/08 11:47:57 changed 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.

07/01/08 11:55:27 changed by bibo

  • attachment jelix-trunk-#561.4.patch added.

add a check on session variable

07/02/08 08:53:26 changed 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 ;)

07/02/08 09:18:20 changed by bballizlife

Oops there already was the review? Sorry ;)

07/09/08 09:26:10 changed 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.

07/09/08 10:23:05 changed by bballizlife

  • review changed from review? to review+.

07/09/08 10:40:56 changed by bibo

  • status changed from new to closed.
  • resolution set to fixed.
Download in other formats: Comma-delimited Text Tab-delimited Text RSS Feed