372966 - (bz-url-extension) Make the URL field into an extension

Status: RESOLVED WONTFIX

(Bugzilla :: Extensions, enhancement, P1)

Description

[Max Kanat-Alexander]

Instead of being a default field, the URL field should be part of a plugin. Most installations don't need a URL field--one of my most common customizations for clients is removing the field from the UI.

However, there are lots of requests here on bmo for adding functionality to the field. So instead of just making it a generic custom field, it should probably become a simple plugin that does some additional things.

Comment 5

[Nitish Bezzala]

I can add a hook to the template to replace the URL field with whatever is in the url hook template. 

But what should I do with the database and the code to add a URL. Should I still use the bug_file_loc column in the bugs table? 

Should the URL now be made a custom field which can be added as a plugin?

Comment 6

[Max Kanat-Alexander]

(In reply to comment #5)
> Should the URL now be made a custom field which can be added as a plugin?

  Yes, exactly. :-)

Comment 7

[hariniachala]

Attachment: Patch to shift URL field code into an extension named BugUrlField

This patch shifts the URL field code to an extension by adding 2 new template hooks. One is added to create.html.tmpl("bug_url_field") and other to edit.html.tmpl ("edit_url_field").

Comment 8

[Max Kanat-Alexander]

  Hey, wow, thanks for the patch! If you'd like to get it reviewed and accepted into the Bugzilla codebase, see our development process here:

  http://wiki.mozilla.org/Bugzilla:Developers

Comment 9

[hariniachala]

Attachment: Resubmission of patch to shift URL field code into an extension

Resubmission of patch to shift the URL field code to an extension by adding 2 new template hooks. One is added to create.html.tmpl("create_url_field") and other to
edit.html.tmpl ("edit_url_field").

Comment 10

[hariniachala]

tnx Max. i read the development process documentation and have resubmitted the patch accordingly.

Comment 11

[hariniachala]

Attachment: BugUrlField extension - an extension to add URL field code to a bug

BugUrlField extension is an extension to add URL field code to a bug. It uses the hooks 'create_url_field' & 'edit_url_field' submitted in previous patch (Attachment 435450 [details] [diff]).

Comment 12

[Frédéric Buclin]

Comment on attachment 435454 [details]
BugUrlField extension - an extension to add URL field code to a bug

A few comments:

- The license block of Extension.pm shouldn't mention EverythingSolved as being the contributor (unless Max paid you to write this extension), and Max and I should't be listed as contributors as we didn't write it. This comment also applies to other templates.

- This extension should add a new type to custom fields, not be a hack for the URL field specifically. This means the URL field should be converted into a custom field, and so would be automatically displayed in the custom fields section of enter_bug.cgi and show_bug.cgi.

Comment 13

[Frédéric Buclin]

Comment on attachment 435450 [details] [diff] [review]
Resubmission of patch to shift URL field code into an extension

Per my previous comment, you should extend the custom field list itself, adding a new type "link" or something like that.

Comment 14

[Max Kanat-Alexander]

  Also, you'll want to attach everything as a single patch, instead of attaching one part as a zip. Making it a single patch makes it much easier for us to review.

Comment 15

[hariniachala]

sorry i have misunderstood the requirements of this bug before. i will write a new patch to make a custom field of type URL. according to the requirements of this bug the custom url field code should run as an extension and not as part of the bugzilla code itself. however there is another bug (https://bugzilla.mozilla.org/show_bug.cgi?id=533604) that requests for the custom url field code to be part of the Bugzilla code itself. can someone please clarify if the custom url field should be implemented as a separate extension or as part of the bugzilla code?

and sorry about the license error. i will make sure it is written correctly next time.

Comment 16

[Frédéric Buclin]

(In reply to comment #15)
> however there is another bug (bug 533604) that requests for the
> custom url field code to be part of the Bugzilla code itself. can someone
> please clarify if the custom url field should be implemented as a separate
> extension or as part of the bugzilla code?

Bug 533604 requests the ability to add several URL fields, which are independent of the existing hardcoded URL field. This bug (372966) is to move the existing hardcoded URL field into an extension/custom field. So I think the right things to do are 1) fix bug 533604, and 2) move the hardcoded URL field into a custom field of type "URL". mkanat?

Comment 17

[Max Kanat-Alexander]

  Umm, the request to add custom URL fields is from just one person--there are no votes and only one CC on the bug. So it doesn't seem like it's that popular of a request. I think we should just move the existing URL field into an extension, and then later, if there are more requests to add custom URL-type fields, we can do that.

Comment 18

[hariniachala]

Attachment: Patch to Field.pm to remove hard coded last added custom field type value and get it from the database instead.

This patch removes the requirement of updating subroutine '_check_type' every time a new custom field type is added by getting the last added custom field type from the database instead of using a hard coded value.

This will make it easier to write an extension for adding a new custom field type because it will remove the requirement of implementing a hook in the '_check_type' method.

Comment 19

[Frédéric Buclin]

(In reply to comment #17)
>   Umm, the request to add custom URL fields is from just one person--there are
> no votes and only one CC on the bug. So it doesn't seem like it's that popular
> of a request. I think we should just move the existing URL field into an
> extension, and then later, if there are more requests to add custom URL-type
> fields, we can do that.

If the URL field moves into an extension, we could as well implement the ability to add URL fields as a new custom field type, no? This sounds more logical to me. So IMO the URL field type should be implemented as part of the extension, i.e. this type shouldn't be hardcoded, but added as part of the extension.

Comment 20

[Frédéric Buclin]

mkanat, any feedback? hariniachala is waiting for you. :)

Comment 21

[Max Kanat-Alexander]

(In reply to comment #19)
> If the URL field moves into an extension, we could as well implement the
> ability to add URL fields as a new custom field type, no? This sounds more
> logical to me. So IMO the URL field type should be implemented as part of the
> extension, i.e. this type shouldn't be hardcoded, but added as part of the
> extension.

  Um, I suppose that's reasonable, although if we're going to do that, we should change the field types from being integers to being string constants, so that extensions can add them without conflicting. That would also allow us to make each object a subclass of Bugzilla::Field, like Bugzilla::Field::Freetext, etc. That should happen in a separate bug (which will have to be filed).

Comment 22

[hariniachala]

(In reply to comment #21)

Is it not possible for the extension to find the highest field type integer value from the db,increment it by 1, and use as the new custom field type value? This method will also help prevent conflicts in new field types added via extensions. Please see the code in the patch I have submitted above to the _check_type method of Bugzilla/Field.pm (this method should also include caching to prevent multiple db calls, as was suggested by reed on IRC).

If this is an acceptable solution I would like to proceed with it:)

Comment 23

[Max Kanat-Alexander]

(In reply to comment #22)
> (In reply to comment #21)
> 
> Is it not possible for the extension to find the highest field type integer
> value from the db,increment it by 1, and use as the new custom field type
> value? 

  No, we may use that value in the future. Also, you'd be depending on convention for Bugzilla to operate properly--a convention that not all extension authors may be aware of, and thus you may get strange and unexpected conflicts.

> This method will also help prevent conflicts in new field types added
> via extensions. 

  But it doesn't present a consistent API--types need to be fixed, particularly for Bug.fields and config.cgi.

Comment 24

[Max Kanat-Alexander]

Comment on attachment 436656 [details] [diff] [review]
Patch to Field.pm to remove hard coded last added custom field type value and get it from the database instead.

  This isn't valid--there's no guarantee that somebody has added a field of the highest type, except coincidentally right now when BUG_URLS happens to be the highest type.

Comment 25

[Kohei Yoshino [:kohei]]

URLs are rather needed more these days.