Skip to content

Commit 883d975

Browse files
authored
[Refactor:System] Set empty preferred names to NULL (#11359)
### Please check if the PR fulfills these requirements: * [ ] Tests for the changes have been added/updated (if possible) * [ ] Documentation has been updated/added if relevant * [ ] Screenshots are attached to Github PR if visual/UI changes were made ### What is the current behavior? <!-- List issue if it fixes/closes/implements one using the "Fixes #<number>" or "Closes #<number>" syntax --> Preferred names can both be empty strings or null to indicate the user does not have a desired preferred name ### What is the new behavior? All existing empty preferred names will be set to null and constraints are added to the database to ensure that empty preferred names cannot be added to the database in the future. ### Other information? <!-- Is this a breaking change? --> <!-- How did you test --> This is part of a series of changes starting with Submitty/SysadminTools#37 and will be followed up by more PRs to remove the handling of empty strings in the database to be able to further simplify the code to not have to handle 2 cases that both mean the user does not have a preferred name.
1 parent 443df76 commit 883d975

File tree

5 files changed

+65
-13
lines changed

5 files changed

+65
-13
lines changed

migration/migrator/data/submitty_db.sql

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -686,6 +686,8 @@ CREATE TABLE public.users (
686686
display_name_order character varying(255) DEFAULT 'GIVEN_F'::character varying NOT NULL,
687687
display_pronouns boolean DEFAULT false,
688688
user_preferred_locale character varying,
689+
CONSTRAINT user_preferred_familyname_not_empty CHECK (((user_preferred_familyname)::text <> ''::text)),
690+
CONSTRAINT user_preferred_givenname_not_empty CHECK (((user_preferred_givenname)::text <> ''::text)),
689691
CONSTRAINT users_user_access_level_check CHECK (((user_access_level >= 1) AND (user_access_level <= 3))),
690692
CONSTRAINT users_user_last_initial_format_check CHECK (((user_last_initial_format >= 0) AND (user_last_initial_format <= 3)))
691693
);
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
"""Migration for the Submitty master database."""
2+
3+
4+
def up(config, database):
5+
"""
6+
Run up migration.
7+
8+
:param config: Object holding configuration details about Submitty
9+
:type config: migrator.config.Config
10+
:param database: Object for interacting with given database for environment
11+
:type database: migrator.db.Database
12+
"""
13+
14+
database.execute("""
15+
UPDATE users SET user_preferred_givenname = NULL
16+
WHERE user_preferred_givenname = ''
17+
""")
18+
database.execute("""
19+
UPDATE users SET user_preferred_familyname = NULL
20+
WHERE user_preferred_familyname = ''
21+
""")
22+
database.execute("""
23+
ALTER TABLE users ADD CONSTRAINT user_preferred_givenname_not_empty
24+
CHECK (user_preferred_givenname <> '')
25+
""")
26+
database.execute("""
27+
ALTER TABLE users ADD CONSTRAINT user_preferred_familyname_not_empty
28+
CHECK (user_preferred_familyname <> '')
29+
""")
30+
31+
32+
def down(config, database):
33+
"""
34+
Run down migration (rollback).
35+
36+
:param config: Object holding configuration details about Submitty
37+
:type config: migrator.config.Config
38+
:param database: Object for interacting with given database for environment
39+
:type database: migrator.db.Database
40+
"""
41+
42+
database.execute("""
43+
ALTER TABLE users DROP CONSTRAINT user_preferred_givenname_not_empty
44+
""")
45+
database.execute("""
46+
ALTER TABLE users DROP CONSTRAINT user_preferred_familyname_not_empty
47+
""")
48+
database.execute("""
49+
UPDATE users SET user_preferred_givenname = ''
50+
WHERE user_preferred_givenname IS NULL
51+
""")
52+
database.execute("""
53+
UPDATE users SET user_preferred_familyname = ''
54+
WHERE user_preferred_familyname IS NULL
55+
""")

site/app/models/User.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@
1717
* @method void setNumericId(string $id)
1818
* @method string getPassword()
1919
* @method string getLegalGivenName() Get the given name of the loaded user
20-
* @method string getPreferredGivenName() Get the preferred given name of the loaded user
21-
* @method string getDisplayedGivenName() Returns the preferred given name if one exists and is not null or blank,
20+
* @method string|null getPreferredGivenName() Get the preferred given name of the loaded user
21+
* @method string|null getDisplayedGivenName() Returns the preferred given name if one exists and is not null or blank,
2222
* otherwise return the legal given name field for the user.
2323
* @method string getLegalFamilyName() Get the family name of the loaded user
2424
* @method string getPreferredFamilyName() Get the preferred family name of the loaded user
@@ -107,17 +107,17 @@ class User extends AbstractModel {
107107
* @var string The given name of the user */
108108
protected $legal_given_name;
109109
/** @prop
110-
* @var string The preferred given name of the user */
111-
protected $preferred_given_name = "";
110+
* @var ?string The preferred given name of the user */
111+
protected $preferred_given_name;
112112
/** @prop
113113
* @var string The given name to be displayed by the system (either given name or preferred given name) */
114114
protected $displayed_given_name;
115115
/** @prop
116116
* @var string The family name of the user */
117117
protected $legal_family_name;
118118
/** @prop
119-
* @var string The preferred family name of the user */
120-
protected $preferred_family_name = "";
119+
* @var ?string The preferred family name of the user */
120+
protected $preferred_family_name;
121121
/** @prop
122122
* @var string The family name to be displayed by the system (either family name or preferred family name) */
123123
protected $displayed_family_name;

site/app/views/UserProfileView.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,10 @@ public function showUserProfile(
2323
string $csrf_token
2424
) {
2525
$autofill_preferred_name = [$user->getLegalGivenName(), $user->getLegalFamilyName()];
26-
if ($user->getPreferredGivenName() != "") {
26+
if (!is_null($user->getPreferredGivenName())) {
2727
$autofill_preferred_name[0] = $user->getPreferredGivenName();
2828
}
29-
if ($user->getPreferredFamilyName() != "") {
29+
if (!is_null($user->getPreferredFamilyName())) {
3030
$autofill_preferred_name[1] = $user->getPreferredFamilyName();
3131
}
3232

site/phpstan-baseline.neon

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11561,11 +11561,6 @@ parameters:
1156111561
count: 1
1156211562
path: app/views/PollView.php
1156311563

11564-
-
11565-
message: "#^Loose comparison via \"\\!\\=\" is not allowed\\.$#"
11566-
count: 2
11567-
path: app/views/UserProfileView.php
11568-
1156911564
-
1157011565
message: "#^Method app\\\\views\\\\admin\\\\ConfigurationView\\:\\:viewConfig\\(\\) has no return type specified\\.$#"
1157111566
count: 1

0 commit comments

Comments
 (0)