If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Add FK constraints to the longdescs table

RESOLVED FIXED in Bugzilla 3.6

Status

()

Bugzilla
Database
--
enhancement
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: Frédéric Buclin, Assigned: Vipin Hegde)

Tracking

(Blocks: 1 bug)

Bugzilla 3.6
Dependency tree / graph
Bug Flags:
approval +

Details

Attachments

(2 attachments, 1 obsolete attachment)

1.23 KB, patch
Max Kanat-Alexander
: review+
Details | Diff | Splinter Review
140.45 KB, image/png
Details
(Reporter)

Description

9 years ago
The bug_id and who columns in the longdescs table should point to bugs.bug_id and profiles.userid respectively.
(Reporter)

Updated

9 years ago
Whiteboard: [Good Intro Bug]
(Assignee)

Comment 1

9 years ago
I can work on this. Is there someone in particular I need to send an email to in order to get this ticket reassigned to me ? Cause I don't see the option to reassign this ticket to myself.
(Reporter)

Comment 2

9 years ago
You can work on it even if not assigned to you. Anyway, the bug is now assigned to you.
Assignee: database → vipinhegde
(Assignee)

Comment 3

9 years ago
Created attachment 362447 [details] [diff] [review]
Patch.diff_V1
(Assignee)

Comment 4

9 years ago
I installed a fresh copy of bugzilla with my changes and verified that the install went through. Also checked the mysql database to verify that the new foreign keys were created successfully. 

I've sent an email to Mark to review the patch, since it is related to DB/Schema.pm.  Should i hold off on marking the ticket as 'Fixed' until after the patch is approved ?
(Assignee)

Updated

9 years ago
Attachment #362447 - Flags: review?(mkanat)

Comment 5

9 years ago
(In reply to comment #4)
> Should i hold off on marking the ticket as 'Fixed' until after the patch is approved ?

You will have to wait your patch to be reviewed+, eventually super-reviewed+ *and* checked into the code base (tree) before marking it as FIXED.
After the patch has been checked into the tree, it has to be verified with the latest codebase to see if nothing is broken. Then, the bug can be marked as VERIFIED.
Status: NEW → ASSIGNED

Comment 6

9 years ago
Vipin, and by the way, don't send email to the people you want a review from: when you are asking "review?" to someone, Bugzilla automatically sends an email to that people.

Comment 7

9 years ago
Comment on attachment 362447 [details] [diff] [review]
Patch.diff_V1

>+++ Bugzilla/DB/Schema.pm	15 Feb 2009 05:22:22 -0000
>@@ -375,8 +375,14 @@
>         FIELDS => [>+            bug_id          => {TYPE => 'INT3',  NOTNULL => 1,
>+				REFERENCES => {TABLE => 'bugs',

  We don't use tabs, we use spaces.

>+            who             => {TYPE => 'INT3', NOTNULL => 1,
>+				REFERENCES => {TABLE => 'profiles',
>+					       COLUMN => 'userid',
>+					       DELETE => 'CASCADE'}},

  I would say RESTRICT for this one. If you've added a comment, you can't be deleted, no? I'm not entirely sure though.
Attachment #362447 - Flags: review?(mkanat) → review-
(Assignee)

Comment 8

9 years ago
I agree with your point on the "RESTRICT" check. I've modified the patch to remove tabs and replaced the CASCADE with the RESTRICT keyword.

- Ran ./runtests . All tests passed.

- Tried out use case by adding one more user to my local bugzilla. Used that new account to enter comments to a bug. After that, I logged in as Admin and tried to delete that user account. Got a valid error message indicating that the user has comments in the system and therefore, cannot be deleted.

Will upload 2 files:
1) Patch.diff  (V2)
2) Screenshot of bugzilla page which indicates that the user cannot be deleted because of pre-existing comments in the system.
(Assignee)

Comment 9

9 years ago
Created attachment 362494 [details] [diff] [review]
Patch.diff_V2

Changelog for this version of the patch.diff file:

1) Replaced tabs with spaces
2) Replaced "CASCADE" with "RESTRICT" for "who" column
Attachment #362447 - Attachment is obsolete: true
Attachment #362494 - Flags: review?(mkanat)
(Assignee)

Comment 10

9 years ago
Created attachment 362496 [details]
Error screen

Screen shot showing error screen UI
(Reporter)

Comment 11

9 years ago
We don't need the screenshot. The page being displayed is unrelated to your patch.

Updated

9 years ago
Attachment #362494 - Flags: review?(mkanat) → review+

Comment 12

9 years ago
Comment on attachment 362494 [details] [diff] [review]
Patch.diff_V2

Looks good to me! :-)

Comment 13

9 years ago
Holding approval for whenever we branch.
Flags: approval?
Target Milestone: --- → Bugzilla 3.6

Updated

9 years ago
Whiteboard: [Good Intro Bug]

Updated

9 years ago
Flags: approval? → approval+
(Reporter)

Comment 14

9 years ago
Checking in Bugzilla/DB/Schema.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB/Schema.pm,v  <--  Schema.pm
new revision: 1.114; previous revision: 1.113
done
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
(Reporter)

Updated

9 years ago
Blocks: 486239
You need to log in before you can comment on or make changes to this bug.