Updated: Proposal for Abstraction Layer for Modifications in

[Gervase Markham]

Matthias Juchem wrote:
> Because of the mails posted on the list, I have redone this text and 
> incorporated some ideas presented in those mails. Please have a look at the 
> result.

I just want to say this is a good idea; it's needed doing for a while. 
Good luck :-)

> To simplify code maintenance I would like to change Bug.pm in a way so that it
> presents one interface for changing bug data that will/can be used by every
> other module that wants to change or read bug data in the database.  

A noble goal; as kiko says, it might not quite work out that way in 
practice. But people always have the option of rolling their own if they 
need to.

>   * Setting the data fields according to the web form fields is done via the
>     set_*/add_* functions mentioned below.

I would instead implement
set(field_name, "value")
get(field_name)

plus a number of
add_*("value")
for things like comments and attachments.

That way, you don't have to change the API of Bug.pm every time we 
change the fields in the bugs table in the database (which happens 
reasonably often), and it'll eliminate a lot of duplicated code. You can 
return a sensible error if the field name is not known.

>     Before updating the data in the database, commit() checks for fields that
>     have to be changed because they depend on other fields that have been
>     changed. These checks will be put in a seperate method to allow easier
>     customization.

Same module as CheckCanChangeField(), perhaps.

>     Therefore commit() also prevents inconsistent partial changes being
>     written to the database.

This sounds fine in theory; but there should be a way of overriding the 
checks in the api e.g. commit(FORCE)) so that things which have no way 
of sending feedback to the user are never messed up.

>   * Bugzilla::Bug::add_attachment() for adding a new attachment
> 
>   * Bugzilla::Bug::add_comment() for adding a new comment
> 

Arguably, set() should not call commit(), but add_*() functions should.

> - Authorization, e.g. CheckCanChangeField from process_bug.cgi, should be
>   moved to a seperate Perl module so that it can be customized easily.
>   This again allow easy customization.

Sounds good. Ideally, it would be like auth now - you can specify the 
module name in params, so people can write their own modules and they 
won't have merge problems.

Gerv