Menu
▾
▴
#9 PxDB prefs auto_approve testing broken
The auto_approve pref, to my understanding, should work
like this:
pref_name: auto_approve
pref_key: datatype ID (integer)
pref_value: 0 for don't auto approve and 1 for do auto
approve.
Universally it appears that it in fact works like this:
pref_name: auto_approve
pref_key: datatype ID (integer)
pref_value: 0 or 1 for do auto approve.
Currently, there is a workaround:
If the pref is not set for a given datatype (i.e.,
there is no key in the auto_approve pref for that
datatype ID), it forces new content to be approved.
While I agree that's a good default, it's probably
worth correcting to behave as one would expect.
Logged In: YES
user_id=568541
The value of the auto_approve pref is not an on/off switch, but rather
_should_ be the guid of the user/group who can auto_approve
content. Setting auto_approve, therefore, to 1 would mean that
content is automatically approved when user 'admin' (assuming that's
ID 1) is submitting it. Setting it to 0 means 'everyone'. etc.
Logged In: YES
user_id=688456
In that case, this behavior:
"If the pref is not set for a given datatype (i.e.,
there is no key in the auto_approve pref for that
datatype ID), it forces new content to be approved."
is broken and should be the bug.
Logged In: YES
user_id=568541
Well, certainly a non empty value for 'approve_disable' will
automatically approve everything. ... but I don't see the logic
hole in 'approve_auto' code, so not sure what the issue is.
The relevant code is in pxdb_commit lines 1109-1126.
$approve_auto_ugids = $this->prefs->get_array
('approve_auto', $this->datatype);
This should _only_ return a non-empty array of values if the
exact match is found. Which means that if there is
no 'auto_approve' for the specified datatype, then the record
wouldn't be approved.
Maybe the problem is elsewhere in the pxdb code ... or
elsewhere in the calling code.
Logged In: YES
user_id=688456
Then the bug isn't in the base classes, it's in Context's
consumption of that information. I'm switching this bug to
Context.
If approve_disable being 0 will approve everything, that's a
bug as well. The expectation in the rest of the db model is
that any switch should be 0 or no value == false, 1 == true.
If it's not a switch, then I don't understand its expected
behavior.
The situation where the global setting approve_disable is
overridden by auto_approve should never occur, IMHO.
Logically, if certain datatypes are to be auto-approved but
everything else is to be approved, then approve_disable
should not be set (or set to 0) and the auto_approve set for
the special datatypes.
Logged In: YES
user_id=568541
Right, bug is probably in Context.
'approve_disable' being 0 will not disable everything. By
"non-empty" I just meant non-empty() :-) The code looks
something like:
if ($prefs->get_pref('approve_disable')) {
...
} elesif ($prefs->get_array('auto_approve')) {
...
}
So ... the situation where approve_disable is overridden by
auto_approve will never arise.
Logged In: YES
user_id=688456
So setting approve_disable to 0 will cause everything to
require approval, because it's not-not-approving. However,
setting a certain datatype to 0 in auto approve will cause
it to automatically be approved, because everybody is
approved even though it's not-not-approved.
Eeeaaagh. OK, not a bug, but maybe a labeling issue.
Logged In: YES
user_id=688456
So setting approve_disable to 0 will cause everything to
require approval, because it's not-not-approving. However,
setting a certain datatype to 0 in auto approve will cause
it to automatically be approved, because everybody is
approved even though it's not-not-approved.
Eeeaaagh. OK, not a bug, but maybe a labeling issue.
Logged In: YES
user_id=568541
Heh :-) yes, I think that's right.
I think of it as basically two options in the system 1)
disable the approval mechanism completely with
'approve_disable' being set to some non-empty() value, or 2)
have the approval system enabled (by not doing the above),
and then selectively [based on datatype] choose which
content (if any) can be automatically approved by certain
users/groups. Yes, the exception is that in the second case
a value of '0' is confusing, because it's a reference to a
ugid ("Everybody"), not to a boolean value.