462068 - Add FK constraints to the longdescs table

Status: RESOLVED FIXED

(Bugzilla :: Database, enhancement)

Description

[Frédéric Buclin]

The bug_id and who columns in the longdescs table should point to bugs.bug_id and profiles.userid respectively.

Comment 1

[Vipin Hegde]

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.

Comment 2

[Frédéric Buclin]

You can work on it even if not assigned to you. Anyway, the bug is now assigned to you.

Comment 3

[Vipin Hegde]

Attachment: Patch.diff_V1

Comment 4

[Vipin Hegde]

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 ?

Comment 5

[Cédric Corazza]

(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.

Comment 6

[Cédric Corazza]

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

[Max Kanat-Alexander]

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.

Comment 8

[Vipin Hegde]

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.

Comment 9

[Vipin Hegde]

Attachment: 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

Comment 10

[Vipin Hegde]

Attachment: Error screen

Screen shot showing error screen UI

Comment 11

[Frédéric Buclin]

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

Comment 12

[Max Kanat-Alexander]

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

Looks good to me! :-)

Comment 13

[Max Kanat-Alexander]

Holding approval for whenever we branch.

Comment 14

[Frédéric Buclin]

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