Forum OpenACS Q&A: SDM: Assigning a patch to a Bug and/or Feature

added support for assigning a patch to a bug and/or feature (BAF) to the SDM, you can see it in action at http://pascal.scheffers.net/sdm. The existing datamodel could not handle this, so I had to add a table, but nothing more:
create table sdm_patch_baf (
        patch_id	 	integer not null references sdm_package_patches,
	baf_id		 	integer not null references bugs_and_features,
	primary key (patch_id, baf_id),

	-- the user who entered the link/patch will be able to alter the 
	-- link as long as it has not been accepted
	user_id		 	integer not null references users,

	-- after accepting ONLY the package, baf and system admins/owners can
touch the link
	accepted_p	 	char(1) check (accepted_p in ('t','f')),
	link_date	 	datetime,

	-- specific notes for this module, if any.
	description	 	varchar(4000)       
);
I only had to add two action pages (for assigning/unassigning BAFs to patches), all the other changes have been done to the existing pages. The 'submit-patch' page now operates in three modes:
  • Do not assign a patch to a BAF
  • Assign the patch to one BAF (http-get)
  • Assign the patch to a bunch of BAFs (http-post)
The 'show-one-patch', 'show-one-baf' and 'show-open-bafs' (I haven't done closed yet) pages have all been updated to allow navigating to and from each other and add or unlink BAFs to the patch. The option to 'accept' patch has been enhanced to (optionally) automatically close the BAFs upon acceptance of the patch. I still need to do some permission checking, but most pages already honor the rules.

The source can be downloaded from my OpenACS workspace, as usual. No patches yet, just everything in a tar ball. It's a drop-in replacement, which works with 3.2.4 and 3.2.5. The only thing you need to do is create the table and restart your AOLServer.

To test it at my site, you can create an account for yourself and just go to the /sdm. To test the admin functions, login as sdmadmin@scheffers.net, password 'sdmadmin'.

Very nice Pascal. Have you tested the "Apply patch and close BAFs" feature to make sure it applies the patch correctly or is able to handle errors?

My selfish question: have you tested this with 3.2.5 from CVS?

Have you tested the "Apply patch and close BAFs" feature to make sure it applies the patch correctly or is able to handle errors?

Yes, I have. The only problem I have is the fact that I don't have the CVS set up correctly, so the CVS specific things fail (nicely, though). The only lines I added to patch-accept.tcl are:
Near the beginning:

if {![info exists bafset]} { set bafset "" }
Which prevents the next piece of code from failing, which I put after everything else (but before return-template):
set bafs [split $bafset]
foreach baf_id $bafs {

    if { $baf_id != 0 && $baf_id != "" } {
        ns_db dml $db "update bugs_and_features set baf_status='3',
completion=(select package_release_id from sdm_package_patches where
baf_id='$baf_id' and patch_id='$patch_id') where baf_id='$baf_id'"

    }
}
There are still some issues I have to resolve before my changes are ready for prime-time, this is one of them: The code above closes the BAFs with the release_id as it is specified in the patch record (the subselect). You should either be able to change the release number for the patch before accepting, updating the patch record (!= possible now) or there should be a way to specify it at 'accept and close BAFs' time. I would prefer the first option, BTW.

My selfish question: have you tested this with 3.2.5 from CVS?
I developed it in 3.2.5 from CVS, although the live demo is running in 3.2.4 (with all SDM files from 3.2.5, obviously).

I found and fixed some other things too, mostly in getting the current user_id. There is one disturbing thing in acs3-pg/tcl/sdm-defs.tcl where the proc sdm_check_admin does this:


    ## HACK TO LET BEN SEE EVERYTHING FOR NOW

    if {$user_id == 4} {
          return 1
Since you have a db handle available there, it must be possible to use a hard-coded email address instead of user_id. Shall I fix that? In that same file, there is a hard coded link to a file in the filesystem:
proc sdm_code_license {} {
    set file [open "/web/aol-dev/www/license.html" r]
This patch has one 'flaw': you cannot assign a patch directly to a module without a BAF. I think it would be great if you could go to a module, click on 'I have a patch for this module' and then go to a submit patch form that will create a BAF record while storing the patch.
Collapse
Posted by Don Baccus on
Uhhh...Ben's hack needs to be cleaned up :)

A fixed e-mail address would be worse, it would give Ben a back-door entry into any SDM on the planet! :)

How about checking for sysadmin or something else along those lines?  Even better would be to investigate to see if Ben got far enough along  with assigning admins etc to let that code disappear entirely.  Easiest would be just to ask him...

I'll let sdm_check_admin return [ad_administrator_p], it doesn't do
anything else anyway (the only thing it does right now is return 1
when the user_id=4). I do think a site-wide-admin should have full
control over the SDM, so that fixup would not cause too much harm
anyway. I'll ask Ben
Collapse
Posted by Ben Adida on
My goodness, that hack is still there?? Geez, that's why I put this
stuff in big letters. Guess I should also tatoo it on my forehead
so I don't forget.

My apologies for this. At least (and this is my excuse) it's clearly
labeled. This should absolutely go away, probably by
designating a fixed way that someone can be SDM global
admin. I don't think this should be linked to site-wide admin: we
want to have SDM admins who are not site-wide.

All he hard coded things will go, consider that done. I'll add an SDM global admin role (or something) and I'll look into the previous postings on this subject (this thread) too.
Collapse
Posted by Don Baccus on
Yes SDM-admins who're not a site-wide admin is necessary, but do make the site-wide admin able to admin the SDM as well!

I get annoyed by some of the ACS 3.x modules that make the sitewide admin add themselves to a special module admin group before they can admin the module (user groups comes to mind).  It's very inconsistent and frustrating.

It's been "fixed" in ACS 4.x - unfortunately by making ONLY the site-wide admin able to do certain package-related admin tasks!