Fix setting ~~DISCUSSION in page.

[pdbogen]

The old semantics were confusing and undocumented. These changes allow the plugin to act as one would expect, where the page-level setting is always used.

Closes #203

[michitux]

From looking at the code, it seems that now weird things will happen when comments are off globally and there is nothing in the page. I think that at least the toggle button will be visible then. I think there should be a check somewhere before the output starts if any non-zero status has been set.

[pdbogen]

Happy to tweak this, but I'm not quite sure I undersatnd your concern.

When you say "there is nothing in the page," do you mean a page that exists but has no content; or a page that does not exist? Or are you referring to a page that just has no discussions?

Also, which "toggle" button are you referring to? The setting on the "discussion moderation" screen, or is there supposed to be a toggle somewhere else?

[michitux]

I mean no comment syntax in the page, but the page exists. At the end of the show function, there is some function called for a toggle button. Iirc this is the button that allows hiding the comments via JS. This button shouldn't be shown when comments are off. As far as I understood your change, this code might now be executed even if comments are off.

[pdbogen]

Hmm. OK, I have discovered a couple edges cases. One is a bug, which I've fixed (not pushed yet): When "automatic" is off, there are no comments, and no in-page directive, _show does run since there's no explicitly false $data['status']. I've fixed this by setting $data['status'] based on isDiscussionEnabled ANY time the page exists, not JUST when isDiscussionEnabled is true.

The other edge case is a little more nuanced, and stems from the fact that there are now two potentially conflicting ways to set discussion status per-page; either through the Moderate Discussions screen (which saves the setting directly to .comments) or via the ~~DISCUSSION~~ directives. If the page has ~~DISCUSSION:off~~ and that directive is removed, the setting saved in .comments is not removed, so discussion remains off. I think this is counter-intuitive.

I think perhaps the setting saved in .comment should have an additional status of "default" where it basically does nothing; and then we can remove the syncing between that setting (via moderate page) and editing page content. The current behavior is otherwise complex and non-intuitive. Thoughts?

[pdbogen]

Alternatively, we can at least make the non-intuitive obvious; perhaps alongside the 'Last modified' text in the metadata section we can add a very unobtrustive comment explaining what DISCUSSION setting is being used?

[pdbogen]

@michitux I'm cleaning up the 'status' stuff overall, trying to centralize the logic to one place and handle the semantics in a straightforward fashion. There's one sticking point, which is that I'm uncertain of what the update_comment_status function is supposed to accomplish. I think it's part of what saves the ~~ tags to the .comments metadata file; I'm inclined to remove this functionality, so that "enabled/disabled via moderator" and "enabled/disabled via page" are completely separate. Do you know a reason why this would be bad?

[pdbogen]

OK. As I said, a bit more invasive, but I think overall everything makes more sense. The pageinfo magic depends on dokuwiki/dokuwiki#1665 though.

[Klap-in]

For normal users this message is not that useful, is it? You could just see if it is enabled or not?

[pdbogen]

I think you're right that it's not useful, but I think it's unobtrusive and not harmful. The target audience for this is casual users (page editors, or administrators that don't care to delve into the inner workings of a plugin); and for them, having the information somewhere obvious seems very valuable.

I could easily add a config option to turn this display off (or enable a restricted version, "enabled/disabled," like you suggest), but even so I'd prefer the default to remain as it is; if someone has gone to the effort to disable the option, they've hopefully done so knowingly, and can be expected to remember to turn it back on if they need the information.

The other (better™, imho) option would be to come up with a really solid story about how the three types of settings interact, so that the behavior is simply intuitive and doesn't need explanation. However, I don't feel that I understand well enough the purpose of the moderator panel to be able to create such a story; so for now I've just tried to make the non-intuitive behavior obvious.

[Klap-in]

The text is at the far bottom of the page. That is not that visible, but therefore indeed not good visible. I had to search to find it, so I wonder if it would reach the right person.

What is the precise use case that needs improvement?

• the user can only use ~~DISCUSSION~~, so for them it doesn't matter which settings are used upstream.
• the admin can make more complicated on/off/close settings, but he has to use the moderator panel for applying this settings.

So I think it could make sense to concentrate on the moderator panel.

What kind of improvements will make the moderator panel more useful for managing off/on/close?
Some first thoughts:
-display what the global setting is,
-show the on-page settings (can these settings be retrieved from the admin page..?).

btw, does the moderator panel override always all ~~DISCUSSION~~ settings? I think it does.

[pdbogen]

Here's the actual problem: Adding or Changing a ~~DISCUSSION~~ tag updates the comments metadata. Removing a ~~DISCUSSION~~ tag doesn't do anything to the comments metadata, and so therefore the page's discussion settings will remain at whatever was the previous setting, which is non-intuitive.

Here's what I would expect to happen:

1. Moderator setting applied; if comments are on/off/closed by moderator, nothing can change this.
2. Page setting applied; if moderator setting unset. Currently, moderator settings cannot be "unset".
3. Global setting applied.

Here's the way it actually works:

1. Page setting applied
2. Moderator setting applied
3. Global setting applied

...and in addition, setting the page setting changes the moderator setting and vice versa.

If "what I think" sounds right, then the moderator panel needs a new "unset" setting, and I can take care of this. It also means removing the two-way sync between the page setting and the comments metadata, which I think is a good simplification.

[Klap-in]

The metadata should just resemble the state of the DISCUSSION syntax tag.
Metadata is for improving accessibility of the info from the page content, and if it is not updated on changes of the page content, something seems not working correct imho. I will have a look for when it is set/unset.

[pdbogen]

OK, then perhaps admin.php should be updated to add a ~~DISCUSSION~~ tag when one doesn't exist (currently it only changes on that's already there); and then I think when a page is saved that has no ~~DISCUSSION~~ the corresponding metadata needs to be cleared, so that the page reverts to the global default.

I still think the semantics of how moderation works are not quite right, if we imagine a separation between "moderator" and "page editor"; because as it is the moderator panel settings do not override the page setting (but instead just update the page setting, which an editor could set back.)

[Klap-in]

Ah, I assumed that moderator panel was overriding the page setting. But that was a wrong assumption.

If the moderator panel setting is same level as page setting, then it should be two-direction-sync to prevent clutter for the user...

About manipulating the page content:

• adding is simple indeed, just add ~~DISCUSSION~~ to content.
• but removing is difficult... because you could not know if ~~DISCUSSION~~ is in a code block. (And converting syntax to instructions and inverse again to syntax is very complicated.)

So there are 3 places where settings are stored:

• global config (as automatic, excluded_ns)
• In serialized *.comments files per wikipage, as status
• on-page with ~~DISCUSSION~~ syntax tag

And buffered:

• metadata of page: as status. Stores just the value of the syntax in metadata (0:off,1:on,2:close)

If metadata of page is parsed from syntax instruction, the update_comment_status() is also called. Reading its code, it shows that this function updates the *.comments file only when status is not zero.
(this corresponds with your observation above)

summary:
current setup where moderator setting is same level as page setting is not well implemented, and it is difficult to improve because on-page setting is difficult to change(without false positives).

[Klap-in]

The admin has the function _changeStatus()
This performs just a find and replace on the ~~DISCUSSION~~ syntax tags in a page if a setting on the moderator panel is changed.

So an design choice of this plugin is not to bother about false positives...

[pdbogen]

Yes, indeed...

But, really, I don't think moderator setting and page-level setting should be on the same level. I think it's simpler for users to understand and simpler in code if we just establish a clear hierarchy: Moderator > Page > Global.

We can repurpose the 'pageinfo' code I'm using to only add a message if the page setting conflicts with the moderator setting; or maybe instead add a warning on the "edit" view; so that the page editor can understand situations where their own setting is ignored.

I think code that changes the raw text (as this does when a change is made from the moderator panel) is dangerous, for example in the situation you mention where it might appear in a code block or, generically, some other context.

[Klap-in]

Every editor can enable/disable-policy
Currently everybody that may edit a page has the choice to disable/enable a discussion. (where eventually the moderator could exclude guests(=not logged in users) from seeing/using discussions)
Note: This is according the base idea of a wiki: that you could trust users, or that otherwise other users fix undesired changes.

moderator can overrule enable/disable-policy
If we change it, so that the moderator overrules the on-page setting, wiki users cannot decide anymore to enable/disable an discussion, if the moderator has blocked this on the moderator panel.

• the global setting is a suggestion for everybody: show default everywhere discussions or not.
• the moderator panel allows to block enabling/disabling
• user with editor rights can enable/disable (if not blocked by moderator)

I expect that if this new policy is introduced, that the old behavoir will be requested: enabling/disabling via admin by moderator, but still changable by user....

Seeing all these consideration/discussions before, I personally like the wiki-like approach. This makes you as user also less dependent on the moderator.

@michitux what are your considerations?

[pdbogen]

Of course, an admin could always just remove the discussion plugin, and had to choose to install it; so there's already a hierarchy amongst server admin and wiki user.

Anyway, if the intent of the moderator panel is just to make it easier to change target pages, then I think there are just some bugs to be squashed around propagate status everywhere it needs to be. I think the fix is to just introduce fourth state for "status" of "unset", and have removing DISCUSSION tags entirely set the status to unset, thus making the global setting apply. We could then keep the "changes the raw text" code.

[michitux]

Some thoughts from me:

• I doubt the original authors intended that there is a hierarchy. The plugin was developed for personal wikis with a public blog, i.e. wikis with basically only one person with edit privileges. I don't know what people are using it for, but I wouldn't care too much about special moderator overrides for the discussion status (and the fact that it is so buggy currently means it can't be used much).
• We can change the discussion plugin syntax safely taking all code/comment/... stuff into account - by using the DokuWiki parser. It is not very easy/obvious, but actually also not terribly difficult once you figured out how it works. I figured it out for the move plugin, that's how the move plugin modifies links. Basically the idea is you let the DokuWiki parser parse the page and use your own custom handler. Your handler then simply outputs what it gets from the parser, unless the parser tells you it's the syntax you want to modify - in which case you modify it. Note that the parser gives you the matched raw text from the page, therefore if you output it directly you do not modify the page in any way, not even whitespace changes are introduced.
• My personal impression of the moderator tool discussion enabling/disabling buttons is that they shall make it easier e.g. to close comments if you receive a lot of spam comments. But I might be wrong, I'm definitely not its heaviest user (and I didn't look at the moderator tool for quite some time).

I'm not sure how to resolve the mess we currently have. In my opinion we should reduce it to one setting per page. We could have it in the page with the syntax and update the syntax through the moderator interface or just have the setting in the comments file and update it with additional elements that can be added to the edit form (similar to the blogtng plugin). I think both has advantages and disadvantages and I'm not sure what we should do to keep backwards compatibility while still making it easier for the user to understand what happens.