MBS-13492: Move beginner status to a privilege flag

This stops us having to calculate beginnership every time, which is slow and cumbersome, and implemented in at least three different places in slightly different ways. Every new editor added via /register will get the BEGINNER_FLAG privilege by default, which will only be removed by a cron job once the number of edits + time passed requirements are fulfilled. We will need to run a one-off script to set the flag on any existing beginners, added as 20240221-mbs-13492.sql. The flag cannot be set/unset via Edit user, since even admins should have no reason to override the default mechanism for this. One small benefit of this is we can better control who is a beginner for testing purposes, so we no longer need to add dummy edits just to allow a test editor to vote.
metabrainz · Feb 21, 2024 · 4784bf7 · 4784bf7
1 parent 1873cac
commit 4784bf7
Show file tree

Hide file tree

Showing 37 changed files with 204 additions and 189 deletions.
diff --git a/admin/sql/updates/20240221-mbs-13492.sql b/admin/sql/updates/20240221-mbs-13492.sql
@@ -0,0 +1,22 @@
+\set ON_ERROR_STOP 1
+
+BEGIN;
+
+UPDATE editor
+   SET privs = privs | 8192 -- set new beginner flag
+ WHERE id != 4 -- avoid setting ModBot as beginner
+   AND NOT deleted
+   AND ( 
+        member_since < NOW() - INTERVAL '2 weeks'
+       OR
+        NOT EXISTS (
+          SELECT 1
+            FROM edit
+           WHERE edit.editor = editor.id
+             AND edit.autoedit = 0
+             AND edit.status = 2
+          OFFSET 9
+        )
+   );
+
+COMMIT;
diff --git a/lib/MusicBrainz/Server/Constants.pm b/lib/MusicBrainz/Server/Constants.pm
@@ -358,14 +358,16 @@ Readonly our $BANNER_EDITOR_FLAG            => 512;
 Readonly our $EDITING_DISABLED_FLAG         => 1024;
 Readonly our $ADDING_NOTES_DISABLED_FLAG    => 2048;
 Readonly our $SPAMMER_FLAG                  => 4096;
+Readonly our $BEGINNER_FLAG                 => 8192;
 # If you update this, also update root/utility/sanitizedEditor.js
 Readonly our $PUBLIC_PRIVILEGE_FLAGS        => $AUTO_EDITOR_FLAG |
                                                $BOT_FLAG |
                                                $RELATIONSHIP_EDITOR_FLAG |
                                                $WIKI_TRANSCLUSION_FLAG |
                                                $ACCOUNT_ADMIN_FLAG |
                                                $LOCATION_EDITOR_FLAG |
-                                               $BANNER_EDITOR_FLAG;
+                                               $BANNER_EDITOR_FLAG |
+                                               $BEGINNER_FLAG;
 
 Readonly our $ELECTION_VOTE_NO      => -1;
 Readonly our $ELECTION_VOTE_ABSTAIN => 0;

diff --git a/lib/MusicBrainz/Server/Controller/Account.pm b/lib/MusicBrainz/Server/Controller/Account.pm
@@ -8,7 +8,7 @@ use namespace::autoclean;
 use Digest::SHA qw(sha1_base64);
 use JSON;
 use List::AllUtils qw( uniq );
-use MusicBrainz::Server::Constants qw( $CONTACT_URL );
+use MusicBrainz::Server::Constants qw( $BEGINNER_FLAG $CONTACT_URL );
 use MusicBrainz::Server::ControllerUtils::JSON qw( serialize_pager );
 use MusicBrainz::Server::Data::Utils qw( boolean_to_json );
 use MusicBrainz::Server::Entity::Util::JSON qw( to_json_array );
@@ -637,6 +637,7 @@ sub register : Path('/register') ForbiddenOnMirrors RequireSSL DenyWhenReadonly
             my $editor = $c->model('Editor')->insert({
                 name => $form->field('username')->value,
                 password => $form->field('password')->value,
+                privs => $BEGINNER_FLAG,
             });
 
             my $email = $form->field('email')->value;

diff --git a/lib/MusicBrainz/Server/Controller/User.pm b/lib/MusicBrainz/Server/Controller/User.pm
@@ -115,7 +115,6 @@ sub serialize_user {
         entityType => 'editor',
         avatar => $user->avatar,
         id => 0 + $user->id,
-        is_limited => boolean_to_json($user->is_limited),
         name => $user->name,
         preferences => {
             public_ratings => boolean_to_json($preferences->public_ratings),

diff --git a/lib/MusicBrainz/Server/Data/Editor.pm b/lib/MusicBrainz/Server/Data/Editor.pm
@@ -46,9 +46,7 @@ sub _columns
 {
     return 'editor.id, editor.name COLLATE musicbrainz, password, privs, email, website, bio,
             member_since, email_confirm_date, last_login_date,
-            EXISTS (SELECT 1 FROM edit WHERE edit.editor = editor.id AND edit.autoedit = 0 AND edit.status = ' . $STATUS_APPLIED . ' OFFSET 9) AS has_ten_accepted_edits,
-            gender, area,
-            birth_date, ha1, deleted';
+            gender, area, birth_date, ha1, deleted';
 }
 
 sub _area_columns { [qw( area )] }
@@ -63,7 +61,6 @@ sub _column_mapping
         privileges              => 'privs',
         website                 => 'website',
         biography               => 'bio',
-        has_ten_accepted_edits  => 'has_ten_accepted_edits',
         email_confirmation_date => 'email_confirm_date',
         registration_date       => 'member_since',
         last_login_date         => 'last_login_date',
@@ -281,6 +278,7 @@ sub insert
             id => $self->sql->insert_row('editor', $data, 'id'),
             name => $data->{name},
             password => $data->{password},
+            privs => $data->{privs} // 0,
             ha1 => $data->{ha1},
             registration_date => DateTime->now,
         );

diff --git a/lib/MusicBrainz/Server/EditQueue.pm b/lib/MusicBrainz/Server/EditQueue.pm
@@ -3,7 +3,16 @@ package MusicBrainz::Server::EditQueue;
 use Moose;
 use DBDefs;
 use MusicBrainz::Errors qw( capture_exceptions );
-use MusicBrainz::Server::Constants qw( :expire_action :editor :edit_status :vote $REQUIRED_VOTES $MINIMUM_RESPONSE_PERIOD $MINIMUM_VOTING_PERIOD );
+use MusicBrainz::Server::Constants qw(
+    :expire_action
+    :editor
+    :edit_status
+    :vote
+    $BEGINNER_FLAG
+    $REQUIRED_VOTES
+    $MINIMUM_RESPONSE_PERIOD
+    $MINIMUM_VOTING_PERIOD
+);
 use DateTime::Format::Pg;
 
 use aliased 'MusicBrainz::Server::Entity::Editor';
@@ -149,6 +158,28 @@ sub _process_open_edit
         $self->log->debug("Applying edit #$edit_id\n");
         unless ($self->dry_run) {
             $self->c->model('Edit')->accept($edit);
+
+            # If the editor is a beginner who can now lose the flag, do that
+            my $editor_id = $edit->editor_id;
+            my $editor = $self->c->model('Editor')->get_by_id($editor_id);
+            if ($editor->is_beginner && !$editor->is_newbie) {
+                my $has_ten_edits = $self->c->sql->select_single_value(<<~"SQL");
+                    SELECT 1
+                      FROM edit
+                     WHERE edit.editor = $editor_id
+                       AND edit.autoedit = 0
+                       AND edit.status = $STATUS_APPLIED
+                    OFFSET 9
+                    SQL
+
+                if ($has_ten_edits) {
+                    $self->c->sql->do(<<~"SQL");
+                        UPDATE editor
+                           SET privs = privs - $BEGINNER_FLAG
+                         WHERE id = $editor_id
+                        SQL
+                }
+            }
         }
     }
     elsif ($status == $STATUS_FAILEDVOTE) {

diff --git a/lib/MusicBrainz/Server/EditSearch/Predicate/Role/User.pm b/lib/MusicBrainz/Server/EditSearch/Predicate/Role/User.pm
@@ -2,7 +2,7 @@ package MusicBrainz::Server::EditSearch::Predicate::Role::User;
 use MooseX::Role::Parameterized;
 use namespace::autoclean;
 
-use MusicBrainz::Server::Constants qw( $EDITOR_MODBOT $STATUS_APPLIED );
+use MusicBrainz::Server::Constants qw( $BEGINNER_FLAG );
 use MusicBrainz::Server::Validation qw( is_database_row_id );
 
 parameter template_clause => (
@@ -48,35 +48,15 @@ role {
         if ($self->operator eq 'me' || $self->operator eq 'not_me') {
             $query->add_where([ $sql, [ $self->user->id ] ]);
         } elsif ($self->operator eq 'limited') {
-            # Please keep the logic in sync with Report::LimitedEditors and Entity::Editor
-            $sql = q{
-              edit.editor != ?
-              AND (
-                NOT EXISTS (
-                  SELECT 1
-                  FROM editor
-                  WHERE id = edit.editor
-                  AND deleted = TRUE
-                )
-              ) AND (
-                  NOT EXISTS (
-                    SELECT 1
-                    FROM edit e2
-                    WHERE e2.editor = edit.editor
-                    AND e2.autoedit = 0
-                    AND e2.status = ?
-                    OFFSET 9
-                  )
-                OR
-                  EXISTS (
-                    SELECT 1
-                    FROM editor
-                    WHERE id = edit.editor
-                    AND member_since > NOW() - INTERVAL '2 weeks'
-                  )
-              )
-            };
-            $query->add_where([ $sql, [ $EDITOR_MODBOT, $STATUS_APPLIED ] ]);
+            $query->add_where([
+              "EXISTS (
+                SELECT 1
+                FROM editor
+                WHERE id = edit.editor
+                AND (privs & $BEGINNER_FLAG) > 0
+              )",
+              [ ],
+            ]);
         } elsif ($self->operator eq 'not_edit_author') {
             $query->add_where([
                 'EXISTS (

diff --git a/lib/MusicBrainz/Server/Entity/Collection.pm b/lib/MusicBrainz/Server/Entity/Collection.pm
@@ -67,7 +67,6 @@ around TO_JSON => sub {
     $json->{public} = boolean_to_json($self->public);
     $json->{description} = $self->description;
     $json->{description_html} = format_wikitext($self->description);
-    $json->{editor_is_limited} = boolean_to_json(defined $editor ? $editor->is_limited : 0);
     $json->{collaborators} = to_json_array($self->collaborators);
     $json->{item_entity_type} = $self->type->item_entity_type if $self->type;
 

diff --git a/lib/MusicBrainz/Server/Entity/Editor.pm b/lib/MusicBrainz/Server/Entity/Editor.pm
@@ -14,7 +14,7 @@ use MusicBrainz::Server::Data::Utils qw(
 use MusicBrainz::Server::Entity::Preferences;
 use MusicBrainz::Server::Entity::Types qw( Area ); ## no critic 'ProhibitUnusedImport'
 use MusicBrainz::Server::Entity::Util::JSON qw( to_json_array to_json_object );
-use MusicBrainz::Server::Constants qw( :privileges $EDITOR_MODBOT);
+use MusicBrainz::Server::Constants qw( :privileges );
 use MusicBrainz::Server::Filters qw( format_wikitext );
 use MusicBrainz::Server::Types DateTime => { -as => 'DateTimeType' };
 
@@ -100,6 +100,12 @@ sub is_banner_editor
     return (shift->privileges & $mask) > 0;
 }
 
+sub is_beginner
+{
+    my $mask = $BEGINNER_FLAG;
+    return (shift->privileges & $mask) > 0;
+}
+
 sub is_editing_disabled {
     (shift->privileges & $EDITING_DISABLED_FLAG) > 0;
 }
@@ -112,7 +118,7 @@ sub may_vote {
     my $self = shift;
 
     return $self->has_confirmed_email_address &&
-           !$self->is_limited &&
+           !$self->is_beginner &&
            !$self->is_bot &&
            $self->is_editing_enabled;
 }
@@ -152,11 +158,6 @@ has [qw( biography website )] => (
     isa => 'Str',
 );
 
-has 'has_ten_accepted_edits' => (
-    is  => 'rw',
-    isa => 'Bool',
-);
-
 use DateTime;
 has [qw( registration_date )] => (
     isa    => DateTimeType,
@@ -195,16 +196,6 @@ has 'preferences' => (
     default => sub { MusicBrainz::Server::Entity::Preferences->new },
 );
 
-sub is_limited
-{
-    # Please keep the logic in sync with Report::LimitedEditors and EditSearch::Predicate::Role::User
-    my $self = shift;
-    return
-        !($self->id == $EDITOR_MODBOT) &&
-        !$self->deleted &&
-        ($self->is_newbie || !$self->has_ten_accepted_edits);
-}
-
 has birth_date => (
    is => 'rw',
    isa => DateTimeType,
@@ -319,7 +310,6 @@ sub _unsanitized_json {
         has_confirmed_email_address => boolean_to_json($self->has_confirmed_email_address),
         has_email_address           => boolean_to_json($self->has_email_address),
         is_charter                  => boolean_to_json($self->is_charter),
-        is_limited                  => boolean_to_json($self->is_limited),
         languages                   => to_json_array($self->languages),
         last_login_date             => datetime_to_iso8601($self->last_login_date),
         preferences                 => $self->preferences->TO_JSON,
@@ -344,7 +334,6 @@ sub TO_JSON {
         deleted => boolean_to_json($self->deleted),
         entityType => 'editor',
         id => $self->id,
-        is_limited => boolean_to_json($self->is_limited),
         name => $self->name,
         privileges => 0 + $self->public_privileges,
     };
@@ -405,21 +394,13 @@ A short custom block of text an editor can use to describe themselves
 
 A custom URL editors can use to link to their homepage
 
-=head2 has_ten_accepted_edits
-
-A flag showing if this user has at least ten accepted non-auto-edits.
-
 =head2 registration_date, last_login_date, email_confirmation_date
 
 The date the user registered, last logged in and last confirmed their
 email address, respectively.
 
 =head1 METHODS
 
-=head2 is_newbie
-
-Determine if this "editor" is a newbie - someone who is new to MusicBrainz.
-
 =head2 is_auto_editor
 
 The editor is an auto-editor
@@ -456,6 +437,10 @@ The editor is able to administer the accounts of other editors
 
 The editor is able to change the banner message
 
+=head2 is_beginner
+
+The editor is a beginner (< 2 weeks old and/or < 10 accepted edits)
+
 =head2 new_privileged
 
 Returns a dummy instance with high editing privileges.

diff --git a/lib/MusicBrainz/Server/Report/LimitedEditors.pm b/lib/MusicBrainz/Server/Report/LimitedEditors.pm
@@ -1,23 +1,15 @@
 package MusicBrainz::Server::Report::LimitedEditors;
 use Moose;
 
-use MusicBrainz::Server::Constants qw( $EDITOR_MODBOT );
+use MusicBrainz::Server::Constants qw( $BEGINNER_FLAG );
 
 with 'MusicBrainz::Server::Report::EditorReport';
 
-# Please keep the logic in sync with EditSearch::Predicate::Role::User and Entity::Editor
-
 sub query { "
 SELECT id,
        row_number() OVER (ORDER BY id DESC)
   FROM editor eor
- WHERE id != $EDITOR_MODBOT
-   AND NOT deleted
-   AND   ( 
-            member_since < NOW() - INTERVAL '2 weeks'
-          OR
-            (SELECT COUNT(*) FROM edit WHERE eor.id = edit.editor AND edit.status = 2 AND edit.autoedit = 0) < 10
-         )";
+ WHERE (privs & $BEGINNER_FLAG) > 0";
 }
 
 __PACKAGE__->meta->make_immutable;

diff --git a/root/collection/CollectionIndex.js b/root/collection/CollectionIndex.js
@@ -25,6 +25,7 @@ import {SanitizedCatalystContext} from '../context.mjs';
 import expand2react from '../static/scripts/common/i18n/expand2react.js';
 import {formatPluralEntityTypeName}
   from '../static/scripts/common/utility/formatEntityTypeName.js';
+import {isBeginner} from '../static/scripts/common/utility/privileges.js';
 import FormRow from '../static/scripts/edit/components/FormRow.js';
 import FormSubmit from '../static/scripts/edit/components/FormSubmit.js';
 import UserInlineList from '../user/components/UserInlineList.js';
@@ -187,7 +188,7 @@ React$Element<typeof CollectionLayout> => {
         {collection.description_html ? (
           <>
             <h2>{l('Description')}</h2>
-            {($c.user || !collection.editor_is_limited) ? (
+            {($c.user || !isBeginner(collection.editor)) ? (
               expand2react(collection.description_html)
             ) : (
               <p className="deleted">

diff --git a/root/components/UserAccountLayout.js b/root/components/UserAccountLayout.js
@@ -18,7 +18,6 @@ export type AccountLayoutUserT = {
   +deleted: boolean,
   +entityType: 'editor',
   +id: number,
-  +is_limited: boolean,
   +name: string,
   +preferences: {
     +public_ratings: boolean,

diff --git a/root/constants.js b/root/constants.js
@@ -69,13 +69,15 @@ export const BANNER_EDITOR_FLAG: 512 = 512;
 export const EDITING_DISABLED_FLAG: 1024 = 1024;
 export const ADDING_NOTES_DISABLED_FLAG: 2048 = 2048;
 export const SPAMMER_FLAG: 4096 = 4096;
+export const BEGINNER_FLAG: 8192 = 8192;
 export const PUBLIC_FLAGS: number = AUTO_EDITOR_FLAG &
                                     BOT_FLAG &
                                     RELATIONSHIP_EDITOR_FLAG &
                                     WIKI_TRANSCLUSION_FLAG &
                                     ACCOUNT_ADMIN_FLAG &
                                     LOCATION_EDITOR_FLAG &
-                                    BANNER_EDITOR_FLAG;
+                                    BANNER_EDITOR_FLAG &
+                                    BEGINNER_FLAG;
 
 export const EDITOR_MODBOT = 4;
 

diff --git a/root/edit/NotesReceived.js b/root/edit/NotesReceived.js
@@ -14,6 +14,7 @@ import {CatalystContext} from '../context.mjs';
 import Layout from '../layout/index.js';
 import * as manifest from '../static/manifest.mjs';
 import linkedEntities from '../static/scripts/common/linkedEntities.mjs';
+import {isBeginner} from '../static/scripts/common/utility/privileges.js';
 import NewNotesAlertCheckbox
   from '../static/scripts/edit/components/NewNotesAlertCheckbox.js';
 import getRequestCookie from '../utility/getRequestCookie.mjs';
@@ -35,7 +36,7 @@ const NotesReceived = ({
     <Layout fullWidth title={l('Recent notes left on your edits')}>
       <div id="content">
         <h1>{l('Recent notes left on your edits')}</h1>
-        {$c.user?.is_limited ? null : (
+        {isBeginner($c.user) ? null : (
           <NewNotesAlertCheckbox
             checked={getRequestCookie(
               $c.req,