Forum OpenACS Development: Re: Cannot delete groups with segments and rels

Posted by tammy m on
Yer right, of course. Posted a patch. I just added code to drop permissions on the group and its rel_segs to your original patch, Randy.

I still think that 2 more additions might need to be made but doubt I am the person to decide +/or do that! Just not intimate enough with the groups system and the OACS design philosophy/decisions on what should be done in pl/sql functions versus tcl procs that call those pl/sql functions. I would opt for more in pl/sql; tcl just calls the db function for ya. That way developers would not have to worry if they "got it all" when they used db functions versus seemingly corresponding tcl procs.

Anyway, here are the 2 other steps I think need to be done to fully delete a group.

In packages/acs-subsite/tcl/group-procs.tcl, group::delete does the extra step (mentioned above in this same thread) of removing rel_constraints.required_rel_segment before calling acs_group__delete (via application_group__delete). I would think the code to delete the rel_constraints.required_rel_segment should be moved to acs_group__delete so that groups could be deleted in one shot via the db function or the tcl proc. But the decision to keep this code in the tcl proc may have been made for design/architecture reasons I'm not grokking yet.

In packages/acs-subsite/tcl/rel-segments-procs.tcl, rel_segments_delete segment_id (which are identified by the rel_constraints.required_rel_segment field as well), does an extra step of deleting dependent constraints before calling rel_segment__delete db function. As above, the code to delete the dependent constraints is not executed if you call the plsql function instead of the tcl proc.

Posted by Andrew Piskorski on
Tammy, I'd say your intuition is right: All functionality needed to properly delete an object should be in the PL/SQL delete function for that object, not spread half and half between the PL/SQL and Tcl code. Some further evidence for this is that the group::delete Tcl proc appears to have an old comment dating from the original 2000 implementation, "Maybe the relational constraint deletion should be moved to the acs_group package..."

So I'd bet good money that the current situation with group::delete is a historical, path of least resistance accident ("well, it's ugly but this way I don't have to write a database upgrade script"), not the result of any careful design.