pkp/pkp-lib#11087 consider masthead options in the old UserForm #11128
Conversation
bozana
force-pushed
the
11087-3_5_0
branch
2 times, most recently
from
aa1b581 to
00911a2
Compare
| @@ -100,6 +100,11 @@ public function scopeWithMasthead(Builder $query): Builder | |||
| return $query->where('user_user_groups.masthead', 1); | |||
| } | |||
|
|
|||
| public function scopeWithMastheadOff(Builder $query): Builder | |||
| { | |||
| return $query->where('user_user_groups.masthead', 0); | |||
There was a problem hiding this comment.
Maybe too late to change this for 3.5.0, but I think when we introduce enumerations, we should use the same values in the DB that we do in PHP-land. This introduces string constants (null, on, off, all) in PHP, but different numeric constants in the DB (1, 0, presumably other values). It would be better just to use one set of constants, even if they're numeric.
There was a problem hiding this comment.
Hmmm... 🤔
These constants are originally meant to be for the user Collector, for filtering there: If one would like to filter by mastheadStatusOff i.e. to get users that are not displayed on the masthead, it would consider both masthead = 0 in the DB table user_groups, as well as masthead = 0 or null in the DB table user_user_groups. Also mastheadStatusAll would get all users no matter what masthead status they have.
But then these constants are also used for user-user-groups functions, but I think this is not necessary.
I can change it, and provide a second commit, and then you can take a look...
Thanks!
bozana
force-pushed
the
11087-3_5_0
branch
4 times, most recently
from
5a4d688 to
c8b6763
Compare
|
Hi @asmecher, I changed the way those enumerations are used, s. second commit. Could you please take a look? |
|
|
|
@bozana, I think something happened to the PRs -- there are a bunch of conflicts and other code now involved. Can you take a look? |
|
Ah, sorry @asmecher, I pushed the wrong branch... 🙈 |
s. #11087