The node moderation system seems to be a bit undervalued. To me a node being in moderation mean it's not ready for public viewing, right?

So, I think it is a significant bug that nodes in moderation show up in the search results and the site-wide tracker.

I don't think it should be very hard to fix this. As far as search results, the relevant code seems to be in node_search():

    case 'search':
      // Build matching conditions
      list($join1, $where1) = _db_rewrite_sql();
      $arguments1 = array();
      $conditions1 = 'n.status = 1';

I still need to try it, but I assume the problem for searching could be fixed by changing the last line above to:

    $conditions1 = 'n.status = 1 AND n.moderate = 0';
CommentFileSizeAuthor
#6 no-moderate.txt1.18 KBpwolanin
#3 moderate_node_track_patch1.txt2.85 KBpwolanin
#2 tracker_moderate_patch0.txt1009 bytespwolanin
#1 search_moderate_patch0.txt462 bytespwolanin

Comments

pwolanin’s picture

pwolanin commented
Title: search returns nodes in moderation » node_search and tracker show nodes in moderation
Component: search.module » node.module
Status: Active » Needs review
StatusFileSize
new search_moderate_patch0.txt462 bytes

patch as per above for node.module. Tested on drupal 4.7.2

pwolanin’s picture

pwolanin commented
StatusFileSize
new tracker_moderate_patch0.txt1009 bytes

patch for tracker.module that only hides nodes in moderation in the site-wide tracker at /tracker

pwolanin’s picture

pwolanin commented
StatusFileSize
new moderate_node_track_patch1.txt2.85 KB

Combined node + tracker patch attached which also checks for moderation when generating the front page or a RSS feed. I think the moderate flag should still be respected in these cases, even if the nodes are marked as promoted. I can readily imagine a workflow for a site where all nodes of a type (e.g. blog) are marked promoted by default, but should not be visible on the front page until approved by a moderator.

Patch is against the 4.7-cvs tarball.

killes@www.drop.org’s picture

killes@www.drop.org commented

I am ok with the patch, anybody disagrees with it?

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Needs review » Needs work

I count 38 queries that need to be looked at if we do this.

> grep -r '{node}' . | grep SELECT | grep status | grep -v moderate

pwolanin’s picture

pwolanin commented
StatusFileSize
new no-moderate.txt1.18 KB

Ok, attached is the list of those 38 with files+ line numbers (4-7-CVS).

I'll try to take a look and make this comprehensive. As per above, it seems to me that the personal tracker didn't need to exclude moderated items, since a user may want to find their own post while it's in the queue. Other case may be similarly not need to be changed.

Also, I made a patch for the event module to have it respect the modeeration flag, if anyone would care to review: http://drupal.org/node/57589

eaton’s picture

eaton commented

I've always been a bit confused by this as well. 'In Moderation' seems like it would make sense as a publication status in and of itself. array(1 => t('Published'), 0 => t('In Moderation'), -1 => t('Unpublished')); or somthing like that.

However, the system as it currently stands implies that 'in moderation' is an additional state that a node can be in, regardless of whether it's published or not. Rather than filtering the display of nodes via the 'moderation' flag, it implies that we should use the flag to determine whether moderation options (additional links, voting widgets, etc) should be presented to users.

I lean towards the first option (moderation as another publication state), personally.

pwolanin’s picture

pwolanin commented

@Eaton: I agree this would make more sense, but sounds like a feature request -> CVS.

As for the 4.7 branch, I think we have to deal the the schema as is.

moshe weitzman’s picture

moshe weitzman commented

-1. the published bit decides if a node is visible or not. after that, i agree that it gets hazy.

basically, the moderation bit should not do anything on its own. it was left there as a bit that modules can use to present queues and so on. the only place node.module should use it is to present a basic moderation queue on admin/node page. from there, admins can choose to publish/unpublish, remove the moderation bit, etc.

my recommendation for 4.8 is to remove all code and DB references to this field and let contrib modules provide their own workflow and db storage. ideally, workflow module is used for this but other modules (queue.module, node_queue, ...) are welcome to provide alternate UI and logic.

dries’s picture

dries commented

The original context of the moderate bit is the queue.module. With the queue.module, nodes that have both the moderate and published bit set were shown to authenticated users, but not to anonymous users. Nowadays we have node-level permission, something which wasn't avaiable when I originally wrote the queue.module. If the same can be achieved with a node-level permission module (and I think it can), it is probably best to take the moderation bit behind the barn, and to shoot it through the head. It's been a pain for years. :-)

Taking out the moderation bit should be both easy, fun and rewarding. SQL queries get less expensive, the user experience is likely to get better, and the code becomes easier to grok.

Maybe check with the workflow people; I believe they had some concrete suggestions about this. Either way, the workflow people figured out a mechanism to build arbitrary workflows with complex transition schemes, and I'm pretty sure they don't need the moderate bit for that. The workflow module is the way forward.

dries’s picture

dries commented

'In moderation' should probably be called 'Draft'. It shouldn't be a checkbox but a button that reads: 'Save as draft'. It's actually a popular feature request. :-) That said, I think that belongs in a separate issue, or maybe not.

pwolanin’s picture

pwolanin commented

Ok, but I'd like to clear this up now for the 4.7 branch. The patch above is pretty minimal, but maybe sufficient. I'm willing to also try to work though the list of other SQL queries if (as it seems) others think it's important, and it's going to be worth my time (i.e. the changes are likely to be committed).

To reiterate, I'm approaching this from the point of view that in 4.7, nodes "in moderation" are not for public consumption, and the core modules should respect this when generating lists for display.

yched’s picture

yched commented

@pwolanin : I don't think you should alter 4.7 behaviour this late. 4.7 updates should only be bugfixes, not modifications of the system behaviour, which could break some existing sites.

pwolanin’s picture

pwolanin commented

Well, to me it's a bug if some core module list/display nodes in moderation and other do not. Why are the book and poll modules different in this respect than the forum or blog modules?! A forum topic in moderation put into a book hierarchy will be visible in some contexts, but not in others....

drumm’s picture

drumm
he/him
Location NY, US
commented

I think we should do the smaller change in 4.7.x- remove it from book and poll.

pwolanin’s picture

pwolanin commented

@drumm: Which means (for example) that handbook pages on Drupal.org will no longer undergo any review?

I think removing it like this would be even more detrimental to functioning sites.

Since the moderation is not set by defualt, and currently has no effect in many contexts, are there sites setting this flag now that would be dirupted by it having an effect?

pwolanin’s picture

pwolanin commented
Version: 4.7.2 » 4.7.x-dev
Status: Needs work » Closed (won't fix)

seems like this is a "won't fix"