[BUG] 5.1.22 upgrade forgets default tags in journal entries

[tfeb]

I use TW as a diary among other things (for which it is wonderful), and I have it set up so that the new journal entries get a log tag by default. This is done simply by setting the default tags in the control panel.

I recently upgraded from 5.1.21 to 5.1.22, using what I think is the standard upgrade process:

1. make a backup;
2. open a downloaded copy of the upgrade TW;
3. point it at my main TW;
4. let it upgrade it;
5. save things and copy to the right place;
6. profit.

This went fine, until today I noticed that my new journal entries did not get the default log tag. And this is because the control panel no longer had it set as a default tag for them. Putting it back in the control panel has resolved the problem.

This is not a major problem since it is easy to fix, but I think the upgrader should not have forgotten configuration like this. It never has before (my current TW originated at 5.1.9 and has been upgraded through most versions since then).

[jeremyredhead]

This is due to a change in how/where default tags are saved from 5.1.21 to 5.1.22 (and then back again in upcoming 5.1.23)

Previously the default tags were stored in the text field of $:/config/NewJournal/Tags, but 5.1.22 used the tags field instead, so the tag picker UI could be used. But this was recently reverted by #4600 due to bug #4591 (the config tiddlers were showing up as being tagged with the default tags... because they were :P)

That's definitely annoying that the upgrader didn't account for that, but it seems like it should be fixable (I say "seems" because I'm not familiar with how exactly the upgrader works)

[tfeb]

Probably the important thing is that the 5.1.23 upgrader copes with TWs that are at 5.1.22: it looks as if it will automagically cope with upgrades from TWs at versions older than that.

[Jermolene]

Thanks @tfeb @jeremyredhead that makes sense. It sounds like the upgrader should detect $:/config/NewJournal/Tags having tags but no text, and move the tags into the text field if so. Is that right @BurningTreeC?

[BurningTreeC]

Yes @Jermolene , sorry for this confusion I introduced in 5.1.22

I don't know if the upgrader should detect those things or if we should leave it as it is with the changes reverted in 5.1.23

[saqimtiaz]

bumping this as we get closer to 5.1.23 @Jermolene @BurningTreeC, so that the upgrade experience from 5.1.22 is as smooth as possible.

[BurningTreeC]

Hi @saqimtiaz , this issue should be resolved

[BurningTreeC]

@saqimtiaz ah yes, we can do something

[Jermolene]

Thanks @saqimtiaz @BurningTreeC do you intend to proceed with the upgrader?

[BurningTreeC]

@Jermolene I thought the fix was using both the text and the tags fields for the tags of new tiddlers. But if the upgrader can fix this problem, it would be better to do so

[Jermolene]

I thought the fix was using both the text and the tags fields for the tags of new tiddlers. But if the upgrader can fix this problem, it would be better to do so

Apologies, I hadn't considered that using both fields might be an option. On balance I guess it keeps things simpler to handle it in the upgrader. One issue is that Node.js wikis cannot use upgraders, so perhaps we should err on the side of caution and do both?

[BurningTreeC]

@Jermolene, I don't know how to handle it in the upgrader, but I can prepare a PR for the wikitext stuff

[Jermolene]

Hi @BurningTreeC did you implement the code to use both the text and the tags fields for the tags of new tiddlers? I think it's the best solution for the moment. If not, could you do so for v5.1.23?

[BurningTreeC]

Hi @Jermolene, yes the code to use both the text and the tags fields for the tags of new tiddlers/journal tiddlers is implemented

[saqimtiaz]

@Jermolene that code is in place, see #5060.

The question is if we also want an upgrader module to resolve the problem as well, as you suggested earlier in this thread?

[Jermolene]

Cool, I'm inclined not to bother with the upgrader because it'll have limited impact given that it doesn't run for Node.js upgrades.

[saqimtiaz]

@Jermolene this is then resolved and can be closed, as well as #4871