Christian, I applied this stuff.
I reviewed the modified files but haven't had a chance to look very
carefully at the added files yet, so expect more on those in the next
day or to. At first glance, I think I'd like to organize the UI for
this a little but, like I said, I haven't really looked at it in much
In applying the patch I made the following changes:
- renamed error constants and put them in the right series
- renamed use_regexp to valid_regexp
- put the load line for the api file in an if block
- renamed custom_field_caption_exists() to
custom_field_is_caption_unique() for consistency with user_api and
- renamed a temporary variable that holds a table name - they never have
"mantis_" in them since it's redundant (minor nit)
- use ON/OFF instead of true/false for config options (not exactly sure
why we do that, but we do :)
Some thoughts or questions that came up as I was reviewing:
- should we use 'name' instead of 'caption'? I find caption kind of weird
- custom_field_create() and custom_field_update() could get passed a
CustomField object (look at how the user_pref_api does it)
- custom_field_bind() seems to be almost exactly the same as
custom_field_add() that was already there...
- custom_field_update(): that's some really really repetitive code! At
very least you could do that by looping over an array of field names.
But I would suggest we do like the user_pref_api and take an object. So
the patter is that you request the field, change what you want, and then
pass it back into the update function. That way the update function can
update everything but only the values you change will actually change.
- I still don't like the dual functionality of custom_field_get_ids().
There are two distinct queries being executed that only share
"db_query($query)". Sharing that line of code isn't enough reason to
put them in one function. I'd like to add, maybe,
custom_field_get_bound_ids( $p_project_id ) or something and make the
existing one not take any parameters
- In manage_proj_edit_page.php:
+ you call helper_alternate_class() with $i but there is no $i defined
+ you keep getting the list of field ids, you could get it once and
use it multiple times (make sure the debugging stuff that prints out the
SQL queries is turned on while you're developing)
+ I'm not sure you should be able to add a new field directly to a
project. It would be nice to separate the interface between defining
fields and adding them to projects. You might want only admins to be
able to define fields, but any developer to be able to add them to a
project, for example. So you can add a field from a list to a project,
or go to a list of fields that are available that allows you to edit
their definitions (that could be scary... making sure the values all
still meet a new regex, for example) or define a new one. Actually, I
would add this as another menu item on the manage page I think: '[
Manage Users ][ Manage Projects ][ Manage Custom Fields ]...' and let
you add fields in the manage projects section
- Why does manage_proj_custom_field_edit_page.php need to take
f_project_id? It isn't used and the field definitions are independent
of the project
More to come, but carry on!
> today was quite productive. I added all the required stuff to manage
> the custom fields.
> So if you run the attached diffs on a CVS version of roughly 8 hours
> ago (but it should work with a recent version, too...) and add
> $g_use_experimental_custom_fields = true;
> to your config_inc.php you should be able to add, update or delete
> custom fields.
> Changes were mainly to managa_proj_edit_page.php to add another table
> for managing the custom fields. This managing ability required also a
> few new pages.
> The custom_fields_api.php also got quite a few modifications. Most
> important were the canging of the mantis_custom_field_table, as it
> contained SQL key-words. I also simplified the custom_field_create()
> very much (we don't need to fill it on creation time; we can easily
> update the custom field definitions later on), and added a few new
> The next day I can work on Mantis again is next Monday. I'd be very
> pleased if these changes/additions can be reviewed (probably fixed) and
> added to the CVS tree till then. So I can continue to work in full pace
> (from a fresh CVS tree nigthly tarball) again and don't need to resync
> the stuff.
> PS: You might want to add a check for $g_use_experimental_custom_fields
> in API.php; I forgot that :(
Beta4 Productions (http://www.beta4.com)