Cannot change capitalisation of products

RESOLVED FIXED in Bugzilla 2.20

Status

()

Bugzilla
Administration
P3
minor
RESOLVED FIXED
17 years ago
5 years ago

People

(Reporter: GavinS, Assigned: GavinS)

Tracking

2.13
Bugzilla 2.20
Bug Flags:
approval +

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Assignee)

Description

17 years ago
This is spun off from bug#64192

Because the comparison is case-insensitive when checking for existing component
and product names, if you try and change the capitalisation of a component from,
say 'test' to 'TEST', bugzilla says the component already exists.
Severity: normal → minor
Target Milestone: --- → Bugzilla 2.16
Created attachment 40265 [details] [diff] [review]
Fix.
The above patch will probably clash with the patch on bug #64192.
Keywords: patch, review
Created attachment 40268 [details] [diff] [review]
Same for products.
Created attachment 40274 [details] [diff] [review]
Let's try it again. (Combined Patch)
Priority: -- → P3
Moving to new Bugzilla product ...
Assignee: tara → justdave
Component: Bugzilla → Administration
Product: Webtools → Bugzilla
Version: Bugzilla 2.12 → 2.13

Comment 6

17 years ago
r=louie@ximian.com

clean patch and solves a problem I've noticed here once before.
Comment on attachment 40274 [details] [diff] [review]
Let's try it again. (Combined Patch)

marking first-review on behalf of louie
Attachment #40274 - Flags: review+
reassigning to patch author
Assignee: justdave → matty
QA Contact: matty → jake
Comment on attachment 40274 [details] [diff] [review]
Let's try it again. (Combined Patch)

>+        if (lc($component) ne lc($componentold)) {
>+            if (TestComponent($product,$component)) {

>+        if (lc($product) ne lc($productold)) {
>+            if (TestProduct($product)) {

I know it's a silly nitpick, but no use introducing more confusion, so we might as well
fix it now. :-)  When you edit a product or component, and *don't* change the name, it
still claims that you updated the product name, even when you didn't.  This is likely to
confuse some people.  Maybe try doing something like:

if (($product ne $productold) && (lc($product) ne lc($productold))) {
Attachment #40274 - Flags: review-
Attachment #40268 - Attachment is obsolete: true
Attachment #40265 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Keywords: patch, review
OS: Solaris → All
Hardware: Sun → All
kiko, I know you like short patches, want a stab at this one?

Comment 11

16 years ago
Sure, if you can explain what you need me to do - this seems kindof fixed to me
with MattyT's patch.
read my last comment. :)  It tells you that you changed the component when you
didn't.
kiko, mattyt: either of you planning to finish this off?

Gerv
We are currently trying to wrap up Bugzilla 2.16.  We are now close enough to
release time that anything that wasn't already ranked at P1 isn't going to make
the cut.  Thus this is being retargetted at 2.18.  If you strongly disagree with
this retargetting, please comment, however, be aware that we only have about 2
weeks left to review and test anything at this point, and we intend to devote
this time to the remaining bugs that were designated as release blockers.
Target Milestone: Bugzilla 2.16 → Bugzilla 2.18

Comment 15

16 years ago
I can't, Gerv; perhaps MattyT is better off right now? :/
(Assignee)

Comment 16

16 years ago
I had a look at this patch, and, with it attached to 2.14:

1) Dave: I couldn't get it to say it had changed the name when I hadn't done
so,  so I don't think the patch needs work in that respect. (comment#9)

2) If Bugzilla is trying to be as helpful as possible, then it should probably
say 'No changes made', if none were.

3) I'm not sure that the patch is the correct way to go. I think the problem is
with the TestProduct() and TestComponent() calls. With the patch applied, and
you try and create a new product named 'TEST', when you already have a product
named 'test', you get the same problem as when changing the capitalisation.

Is it possible to change the SQL comparison to be case-sensitive? I think you
can do this by changing the data type, but I'd hope that wouldn't be necessary.


(Assignee)

Comment 17

16 years ago
I've updated the title, because I think the same issue arises (on 2.14 anyway)
for the keywords and groups :(

I can raise new bugs for these, if that would be easier?

I also tried a different fix to the currently attached patch for the original
component/product issue. This used SQL to select all the components which
matched the new component name and then Perl to check, case-sensitively, for
matches.

This approach worked OK for adding new components whose name only differed in
capitalisation to existing component names, and for changing capitalisation,
but, of course, you then have problems selecting which component you want to edit :(

If you want Bugzilla to be able to have 2 components whose names only differ in
capitalisation, then I think that the bug for using component_ids instead of
component names needs to be fixed before this bug can be. (can't remember the
bugid for that one at the mo.)
Summary: Cannot change capitalisation of Components or Products → Cannot change capitalisation of Components/Products/Keywords/Groups
*** Bug 125683 has been marked as a duplicate of this bug. ***
Attachment #40274 - Flags: review+
(Assignee)

Comment 19

15 years ago
*** Bug 140348 has been marked as a duplicate of this bug. ***
Taking Jake's bugs...  his Army Reserve unit has been deployed.
QA Contact: jake → justdave
All 2.18 bugs that haven't been touched in over 60 days and aren't flagged as
blockers are getting pushed out to 2.20
Target Milestone: Bugzilla 2.18 → Bugzilla 2.20
(Assignee)

Comment 22

14 years ago
Created attachment 150303 [details] [diff] [review]
Allow product capitalisation changes

This just deals with products. It allows product capitalisation changes, and
also gives an error when a new product differs only in case from an existing
one
(Assignee)

Updated

14 years ago
Assignee: mattyt-bugzilla → bugzilla
(Assignee)

Updated

14 years ago
Attachment #150303 - Flags: review?

Comment 23

14 years ago
Comment on attachment 150303 [details] [diff] [review]
Allow product capitalisation changes

>Index: editproducts.cgi
>+    # Check for exact case sensitive match:
>+    if ("$existing_product" eq "$product") {

Nit: kill the quotes around the variables; they're really unnecessary.

>+    if (lc("$existing_product") eq lc("$product")) {

Same here, although I'd be ready just do this latter test; it's quite ok to say
"The product name 'xxx' is already in use" even if there is a case-only
difference. Humans don't consider that a difference.

>+        if (lc($product) ne lc($productold)) {
>         if (TestProduct($product)) {
>             print "Sorry, product name '$product' is already in use.";
>             SendSQL("UNLOCK TABLES");
>             PutTrailer($localtrailer);
>             exit;
>         }
>+        }

Combine the conditions using && instead of this unindented nested if.
Attachment #150303 - Flags: review? → review-
(Assignee)

Comment 24

14 years ago
Created attachment 153340 [details] [diff] [review]
Allow product capitalisation changes (V2)
Attachment #150303 - Attachment is obsolete: true
(Assignee)

Updated

14 years ago
Attachment #153340 - Flags: review?(jouni)
(Assignee)

Comment 25

14 years ago
(In reply to comment #23
> Nit: kill the quotes around the variables; they're really unnecessary.

Done (too much bash programming!)

> 
> >+    if (lc("$existing_product") eq lc("$product")) {
> 
> Same here, although I'd be ready just do this latter test; it's quite ok to say
> "The product name 'xxx' is already in use" even if there is a case-only
> difference. Humans don't consider that a difference.

I've kept the test, if that is OK -- as I'm sure there will be one day when someone tries this and 
complains if we don't do this test.

> Combine the conditions using && instead of this unindented nested if.

Done

Comment 26

14 years ago
Comment on attachment 153340 [details] [diff] [review]
Allow product capitalisation changes (V2)

r=jouni
Attachment #153340 - Flags: review?(jouni) → review+

Comment 27

14 years ago
GavinS: File separate bugs on Components, Keywords and Groups; mention the bug
ids here so that people can track the discussion. Thanks!
Flags: approval?
Summary: Cannot change capitalisation of Components/Products/Keywords/Groups → Cannot change capitalisation of Products
(Assignee)

Comment 28

14 years ago
Created bug#252003, bug#252004, and bug#252005 for the components, keywords and groups issues
Flags: approval? → approval+

Comment 29

14 years ago
Checking in editproducts.cgi;
/cvsroot/mozilla/webtools/bugzilla/editproducts.cgi,v  <--  editproducts.cgi
new revision: 1.55; previous revision: 1.54
done

Thanks again, Gavin!
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
Summary: Cannot change capitalisation of Products → Cannot change capitalisation of products
QA Contact: justdave → default-qa
You need to log in before you can comment on or make changes to this bug.