Closed Bug 234175 Opened 16 years ago Closed 16 years ago

Remove deprecated ConnectToDatabase() and quietly_check_login()/confirm_login() calls


(Bugzilla :: Bugzilla-General, defect)

Not set



Bugzilla 2.18


(Reporter: wicked, Assigned: wicked)




(1 file, 2 obsolete files)

Need to remove calls to ConnectToDatabase() because it's deprecated and actually
does nothing. Database connection is established automatically in

Uses of quietly_check_login() and confirm_login() will be changed to use
Bugzilla->login directly.

I think this is better to do in one big patch than to gradually change call when
individual scripts are touched. This way we get rid of old code that just
confuses things once and for all. Besides, this is simple enough change to get
used hacking Bugzilla..
Attached patch Changes, v1 (obsolete) — Splinter Review
This patch removes redundant login in attachment.cgi and so should fix bug
Attachment #141377 - Flags: review?
Ever confirmed: true
Target Milestone: --- → Bugzilla 2.18
Comment on attachment 141377 [details] [diff] [review]
Changes, v1

Since bbaetz wrote the Auth code, he's the best person to look at this
Attachment #141377 - Flags: review? → review?(bbaetz)
Comment on attachment 141377 [details] [diff] [review]
Changes, v1

You can leave out LOGIN_NORMAL; its teh default. I didn't do that in the compat
code because of the ?: - it was clearer when mentioned explicitly.

You don't need |use Bugzilla::Constants| in all cases then, either
Attachment #141377 - Flags: review?(bbaetz) → review-
Attached patch Revised changes, v2 (obsolete) — Splinter Review
Attachment #141377 - Attachment is obsolete: true
Attachment #141400 - Flags: review?(bbaetz)
Attachment #141400 - Flags: review?(kiko)
Comment on attachment 141400 [details] [diff] [review]
Revised changes, v2

>Index: enter_bug.cgi
>-confirm_login() if (!(AnyEntryGroups()));
>+Bugzilla->login(LOGIN_REQUIRED) if (!(AnyEntryGroups()));

Nit: I would rather you had fixed this to use unless, but it's not a blocker; I
can do that before checking in if you like.

r=kiko after a 15-minute read.
Attachment #141400 - Flags: review?(kiko) → review+
Dave, this touches a lot of files, though it's fairly trivial. I've had a good
look over it and there's nothing dangerous that I've found. Requesting approval
so we can have it in sooner than later.
Flags: approval+
Flags: approval+ → approval?
I want to wait for bbaetz's review.
Flags: approval?
Blocks: 149167
Comment on attachment 141400 [details] [diff] [review]
Revised changes, v2

This looks fine.
Attachment #141400 - Flags: review?(bbaetz)
Flags: approval?
Flags: approval? → approval+
Attached patch kiko_unrotSplinter Review
Unrotted this -- minor rejects that were easily fixed. This is what is going to
be checked in.
Attachment #141400 - Attachment is obsolete: true
Checked in -- thanks to Teemu for this great cleanup.
Closed: 16 years ago
Resolution: --- → FIXED
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.