The regexp in Bugzilla::BugUrl::JIRA::should_handle() isn't restrictive enough (min two letters required)

RESOLVED FIXED in Bugzilla 4.2

Status

()

--
minor
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: mail, Assigned: mail)

Tracking

Bugzilla 4.2
Bug Flags:
approval +
approval4.2 +

Details

Attachments

(1 attachment)

(Assignee)

Description

7 years ago
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:9.0.1) Gecko/20100101 Firefox/9.0.1
Build ID: 20111223083612

Steps to reproduce:

Current code is

sub should_handle {
    my ($class, $uri) = @_;
    return ($uri->path =~ m|/browse/[A-Z]+-\d+$|) ? 1 : 0;



Actual results:

JIRA allows you to specify the regex that a project can be. The default is two or more lower case letters. However in JBoss JIRA the regex is [A-Z][A-Z]+[0-9]{0,2}

http://confluence.atlassian.com/display/JIRA044/Configuring+Project+Keys


Expected results:

There are three possible solutions:

1) Red Hat carry the change ourselves
2) The regex above is changed to m|/browse/\w+-\d+$|
3) We add this as a param value with [A-Z]+-\d+ as the default

I'm in favour of the last one, but wanted your input before writing the code and submitting a patch.

Comment 1

7 years ago
(In reply to Simon Green from comment #0)
> I'm in favour of the last one, but wanted your input before writing the code
> and submitting a patch.

No, we won't accept a parameter for such a minor thing. If JIRA says the correct regexp is  m|/browse/\w+-\d+$|, then we should use it.
Assignee: general → create-and-change
Severity: normal → minor
Component: Bugzilla-General → Creating/Changing Bugs
(Assignee)

Comment 2

7 years ago
(In reply to Frédéric Buclin from comment #1)
> No, we won't accept a parameter for such a minor thing.

Thought as much.

> If JIRA says the
> correct regexp is  m|/browse/\w+-\d+$|, then we should use it.

The correct regex for the is |/browse/[A-Z][A-Z]+-\d+$| (i.e. two or more upper case letters, source http://confluence.atlassian.com/display/JIRA044/Configuring+Project+Keys). I have created a patch for this.

Red Hat will carry the change to handle our JIRA server ourselves.
(Assignee)

Comment 3

7 years ago
Created attachment 587673 [details] [diff] [review]
v1 patch
(Assignee)

Updated

7 years ago
Attachment #587673 - Flags: review?

Comment 4

7 years ago
Comment on attachment 587673 [details] [diff] [review]
v1 patch

timello: this one is for you. Why not using [A-Z]{2,} instead of [A-Z][A-Z]+ ?
Attachment #587673 - Flags: review? → review?(timello)
(In reply to Frédéric Buclin from comment #4)
> Comment on attachment 587673 [details] [diff] [review]
> v1 patch
> 
> timello: this one is for you. Why not using [A-Z]{2,} instead of [A-Z][A-Z]+
> ?

Yeah, it can be [A-Z]{2,}. I'm going to test it and r+ the patch. We fix it on checkin.
Comment on attachment 587673 [details] [diff] [review]
v1 patch

It looks good and it works as expect. We can either use [A-Z][A-Z]+ or [A-Z]{2,}. However, I think that the first one is more readable, unless there is a good reason to use the last one.
Attachment #587673 - Flags: review?(timello) → review+
Flags: approval?

Updated

7 years ago
Assignee: create-and-change → sgreen+mozilla
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: approval?
Flags: approval4.2+
Flags: approval+
Target Milestone: --- → Bugzilla 4.2

Updated

7 years ago
Summary: Bugzilla::BugUrl::JIRA should_handle should be configurable → The regexp in Bugzilla::BugUrl::JIRA::should_handle() isn't restrictive enough (min two letters required)
Committing to: bzr+ssh://timello%40gmail.com@bzr.mozilla.org/bugzilla/trunk/                                                                                                                                    
modified Bugzilla/BugUrl/JIRA.pm
Committed revision 8096.

Committing to: bzr+ssh://timello%40gmail.com@bzr.mozilla.org/bugzilla/4.2/                                                                                                                                      
modified Bugzilla/BugUrl/JIRA.pm
Committed revision 8015.
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.