-
Notifications
You must be signed in to change notification settings - Fork 19
Prevent email truncating by increasing size of username
column in users
table.
#186
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
381e76d
3e27b27
84212de
239a494
11ef538
dbf8c9b
810d761
39a1b09
be9266d
11783fd
4f5e4b6
3dba4de
343d327
1409d02
35ce784
bf5afd1
8c9c8e1
2902e61
c18d5a7
bac91a9
09e058f
0bf72aa
b3ae400
3da36fb
920e4a9
2296095
481e3aa
8d0e99a
09456ce
2ff4a33
9202147
5fcb073
4037ce2
f7750ee
ab686bc
0125129
ff132d6
871e89f
22d2de9
477c306
0f56350
91e8034
bb4a6b2
f9e4d6c
1090536
ed9a857
49b68a7
2e351fa
462fad2
c58098f
af2d04f
ba899f2
54afc6b
a2328bd
26f6d55
b5d8a70
77394ba
e24a816
7de06d5
9edfc4f
ec6e818
c7bdf04
3a28fa8
87a8eaf
7644dcf
db9400c
1c880ed
5ea765f
cdb627e
fbb05f8
e489ac2
b947406
7ff0b7d
6377ae4
4901347
2495969
66a9fe7
3912bce
59c09e0
5fe2f72
1ea9c7f
55fd0c0
857b013
484d1dd
d5565f7
cdd03cc
6142295
c500eb0
0be3526
2314a9b
3bc038e
d9ffdb1
d6f4cc0
d3d41ef
edd5986
20f8d99
ad9c657
99406a7
6b74f5f
d1503cb
a9d8912
b1e4c56
6abb16d
c530bbb
56fd67e
ee53c53
a0cdead
15877da
bf419a6
e1ce1d8
1d82d97
2e8bf7d
b7be9cd
8b6b297
ee0dfc8
799a38e
ab23142
2254566
f788e8b
2d2d18f
c54cd5d
ae22388
e103eff
43eaf01
cd6b0c8
0692f77
09ea9f0
c438855
1178841
a87f9a6
556fb8b
05c5e5f
dd64392
7d636f1
6b43c09
be2a457
d963bc2
9577d27
56406ee
477a963
097616c
43b7422
282cf29
1f7449c
1e18f69
450f55e
76e5668
cfb6421
7bade19
40f3fad
24394d2
3a92bde
4d4a89a
465b2a3
d882d55
8545251
80f420b
bf6ea06
fb0b8f0
78c907a
1e68f2c
0468195
32df3dd
aac734c
b331019
c95c0dd
df7cf20
1a53562
cdac02b
a33293a
82d7d60
11fc0d6
890249f
f2de9ac
a39d3ba
f923881
1c7b228
6ba68d9
6dc7adc
5d8aac6
b1fcd44
06ff5c2
7ff3e83
bc9b830
54e9d02
eebf936
3962e6a
c84ddeb
f9cce8b
9d5cb79
e689ae7
f56d512
d6fd287
00fb172
684dfd0
7f8a56b
55eabf9
319369e
7768529
21e0b1f
faedb36
57bc49b
40fc6b0
fb44549
f9977f8
4a292e6
fa52e26
2096bbe
df03dc6
62508bf
5b84bb2
8453edb
3b43694
51154bc
7ba7c12
de2a15f
4032dc1
55ed1aa
1cf2aa0
4dc8fe8
54917bb
21ed34f
93bd5ea
8d63fd2
7ef4c0d
60b3497
43e3d72
5a2c288
cef4eec
fcfbd8f
4541621
73c5133
d51c465
29d0af1
cbe802b
f5888f8
11e0b3c
cdc75e2
2c930f2
2749fdc
06dc3a7
2722b9e
fb2b62c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -50,10 +50,19 @@ public function up(): void | |
'type' => 'VARCHAR', | ||
'constraint' => '1000', | ||
'null' => true, | ||
], | ||
] | ||
]; | ||
|
||
$this->forge->addColumn('users', $fields); | ||
|
||
$fields2 = [ | ||
'username' => [ | ||
'type' => 'VARCHAR(256)', | ||
'null' => true | ||
] | ||
]; | ||
Comment on lines
+58
to
+63
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix formatting issues and consider whether modifyColumn is the right approach. There are several issues with this code block:
$fields2 = [
'username' => [
'type' => 'VARCHAR(256)',
- 'null' => true
+ 'null' => true,
]
]; Consider using try {
// Try to modify if exists
$this->forge->modifyColumn('users', $fields2);
} catch (\Exception $e) {
// Add if it doesn't exist
$this->forge->addColumn('users', $fields2);
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The field as created by CI Shield is nullable, so I maintained that. I would normally not modify this attribute at all, but CI docs for migrations state that nullable is buggy and should be set every time to ensure expected outcome. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @evansharp Thanks for clarifying about maintaining the nullable attribute - that makes sense and aligns with what we already know about CodeIgniter Shield's default behavior. What about the other issues mentioned in the original comment? Specifically:
Would you like to address these points as well? ✏️ Learnings added
🧠 Learnings used
|
||
|
||
$this->forge->modifyColumn('users', $fields2); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If a developer has already run the migration, executing it again will have no effect because the migration system only detects new migrations. Therefore, it's best to create a new migration file and apply the necessary changes there, according to what you have done. This ensures that your changes are properly applied to the database without modifying previous migrations. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think that is a problem though. Any developer already using the library will already have handled this short field somehow; if anything, adjusting their table could break their fix. I think it is something only necessary for a new installation and so updating the existing migration is all that is needed. |
||
} | ||
|
||
public function down(): void | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix the trailing comma formatting issue.
There's a formatting issue reported by PHPCSFixer regarding missing trailing commas.
🧰 Tools
🪛 GitHub Actions: PHPCSFixer
[error] 56-56: PHP CS Fixer: trailing_comma_in_multiline, binary_operator_spaces issues found. Please fix the formatting.