Forum OpenACS Q&A: SDM Changes

Collapse
Posted by Roberto Mello on
I made some changes to the SDM, fixed some things and improved others.
The problem I was having is that after I assigned a project to a team,
I wanted that project to be private, meaning only visible to those in
the team, not anybody else.

To do that, I created another table that maps packages to user groups
(teams) and modified a number of SDM authorization procs. On the way
I fixed some issues (like sdm_check_admin).

Ben, do these changes look allright to you? If not, let's discuss them
and see what we can agree to.

Collapse
2: SDM Changes: Need feedback (response to 1)
Posted by Roberto Mello on
The changes I have made to SDM basically deal with the way permissions are done. I don't understand everything behind the system but this is what I thought:

  • Teams are created (e.g: teamA, teamB)
  • Users are assigned to Teams
  • Packages are assigned to Teams (e.g: cms -> teamA, templating -> teamB)
  • Project admin assigns members to roles (e.g: John (teamA) -> internal)
  • Project admin assigns actions to roles (e.g: internal members can view project, edit project and fix issue. external can view project only)

Therefore:

  • Members of the team that owns the package (granted that they have "view_project" permission) should be allowed to view the project _even_ if the project is not public.
  • By having packages assigned to teams we won't have to create a team for every new package
  • this will give us better granularity on the permissions and more flexibility.

To accomplish this, I created one table:

CREATE TABLE sdm_package_user_group_map (
   package_id  integer 
      constraint sdm_pac_ug_map_package_id_fk
      references packages(package_id),
   group_id    integer 
      constraint sdm_pac_ug_map_group_id_fk
      references user_groups(group_id)
);
and did a modification to user_can_see_package(user_id,package_id), plus modified the pages in the authentication part (for example, in the index.tcl, users will see packages that are public AND those that belong to the group that he/she belongs to).

Comments??? Please.

Collapse
3: Response to SDM Changes (response to 1)
Posted by Roberto Mello on
Oh, some other things I want to change:

Right now AFAICS, only package admins can assign bugs/features to team members. I'd be hesitant to give someone package admin priviledge just so he can assign tasks to others. This has lead to a huge number of non-assigned bugs/features on the OpenACS SDM.

I'd like to be able to setup another action: assign_issues. So anyone with assign_issues priviledge, even a non-admin, could assign tasks to other people.

This would give us the possibility of having someone with Bruce Momjian's role kind of thing. He doesn't code, but he helps keep the effort organized and people focused on tasks. One thing that we certainly have missed in the OpenACS project (since Ben can't do _everything_).

Other things I am ready to do (if Ben/others think I should):

  • Make the fix process a little faster. Right now you have to do too many click to be able to do it.
  • When you close an issue, you go back to the Open issues, instead of closed ones (as it is now). Going to closed issues after closing one is a pain when you are closing a bunch of issues at a time.
  • Give bug/feature reporters the ability to enter HTML code. I had to "view source" many times to be able to view the report correctly, when closing OpenACS 3.2.5 issues this week.
  • Give bug/feature reporters the ability to submit a patch when submitting a report. That way you could see exactly which patch fixes what bug/implements which feature.
  • Improve the CVS code. I have been looking into viewscvs' source code to learn how it works. viewcvs is a cvsweb clone, but written in Python. I can NOT, for the life of it, understand cvsweb's Perl code (I tried. I probably could, but it'd take me 500 more hours).

Phew! that's in for now. Please add your comments/suggestions here while I am up to it. It'd be really nice to get these improvements in so we can use them on OpenACS 4 and OpenNSD (and my other projects :))

Collapse
4: Response to SDM Changes (response to 1)
Posted by Ben Adida on
Some good ideas here. Off the top of my head:
  • I'm not sure whether the role should be something so specific as assign_issues. Yes, it's important that people (other than admins) be able to assign issues. The way the ticket tracker works, and the way this should probably be structured, too, is with three roles: external (just adding bugs), internal (developing code, assigning/fixing bugs), and admin. We can rename these roles to something more obvious, but I think those 3 levels of permission are specific enough.
  • For submitting patches, try using the existing patch data model / code, so that a patch can be linked to a bug.
  • CVS improvements: what are you thinking of doing there?
Collapse
5: Response to SDM Changes (response to 1)
Posted by Roberto Mello on
"be structured, too, is with three roles: external (just adding bugs), internal (developing code, assigning/fixing bugs), and admin. We can rename these roles to something more obvious, but I think those 3 levels of permission are specific enough."

Ok, I see the point now and I agree. But the code does not respect that. For example, right now in baf-assignments.tcl (to assign issues to someone) it calls user_can_edit_package_p, which only returns true if the user is a package admin. It doesn't even check if you are internal or not.

user_can_edit_package_p is probably doing what it's intended to do. But then we should have another proc, like user_can_assign_issues_p or just a query to check if the user is internal. In other words, we should change the permission model to check for the roles/actions (I did that to user_can_see_package_p).

"For submitting patches, try using the existing patch data model / code, so that a patch can be linked to a bug."

The existing patch data model doesn't allow that:

create table sdm_package_patches (
       patch_id                  integer not null primary key,
       user_id                   integer not null references users,
       package_id                integer not null references packages,
       package_release_id        integer not null references package_releases,
       submission_date           datetime,
       patch_file                varchar(200),
       patch_description         varchar(4000),
       accepted_p                char(1) check (accepted_p in ('t','f')),
       action_user               integer references users,
       action_date               datetime,
       action_description        varchar(4000)
);

"CVS improvements: what are you thinking of doing there?"

Mostly interface issues. For example, I like the way cvsweb/viewcvs shom me which lines have been added/removed/changed. It helps to quickly identify a change, learn from it, or spot an error introduced in a revision.

I haven't studied the CVS portion of SDM very much, but I think it puts somethings in the DB, and the admin has to manually update the CVS on SDM, which is not doable with something as dynamic as a CVS tree.

What do you think about the idea of assigning projects to teams?

Collapse
6: Response to SDM Changes (response to 1)
Posted by Roberto Mello on
Continuing on SDM changes discussion, I'd like to discuss some problems I've seen and propose solutions (I am putting this on a table, so the e-mail format will probably look crappy. My apologies). Note that these are my opinions only, so please speak up:

Problem Proposed Solution
Projects are not owned by a team. This (among other problems) causes private projects to be private to everybody, including those that should be working on it. Add an extra table to map projects to teams.
Current permissions code does not honor roles Fix permissions code to honor roles.
Assigning issues to people takes too long. You have to enter name/e-mail, click, wait, click Upon issue closing, present a drop down box with names of those in the team AND the box for searching for others.
Closing issues one-by-one takes too long and is boring. Add text box where you can enter baf numbers separated by spaces, to be bulk-closed.
Comments on issues are hard to visualize if code is being posted Add HTML option for comments
There's no esay way to see which issues have no patches attached to them. Add a link that will show issues without patches (suggested by someone else)

That's all I can think of now. Hopefully we can get this done soon, as I see it as something important for OpenACS (both 3.x and 4.x)

Collapse
7: Response to SDM Changes (response to 1)
Posted by Don Baccus on
These look good.  For bulk closing of bafs, how about a checklist form
rather than typing in the numbers by hand?  Personally I can't
remember more than a handful of numbers at a time...
Collapse
8: Response to SDM Changes (response to 1)
Posted by Pascal Scheffers on
I was about to send a large patch for the SDM this monday, when I
realized I needed to add an edit patch to make bulkclosing BAFs
possible. As I am still waiting for acs-messaging, I can do the
proposed things above (or some of them anyway). sdm-patch-a-baf is
95% done. Does it look good enough for you, or would you like it to
do more?
Collapse
9: Response to SDM Changes (response to 1)
Posted by David Kuczek on
I have some suggestions:

1.) Copy the good things out of bboard - and bboard really works...

1.1.) Make the first page of sdm very similar to bboard. You see the title, posting-date, and the author of n-bugs on the first page. In brackets you could see if this bug has already a patch or not.

1.2.) Enable alerts. This way people know that a certain amount of regular visitors of openacs get that bug or the patch immediately. Additionally people can set a specific alert at a certain bug report that interests them.

1.3.) Show all new patches (parallel to show all new answers).

1.4.) Show all bugs that don't have a patch.

2.) Then it would be nice to have a flag at every bug that shows you if that bug has been fixed inside cvs. This way you could at one glance see what is new inside cvs. -- One more thing that is concerning all of ACS: We changed util_convert_plaintext_to_html, because you have to hit the "return" key twice while writing something in a textarea in order to display a new paragraph. Our solution is that every "return" is displayed like a regular new line.

We changed every "p tag" into a "br tag" and split all the numbers after "regsub -all" in half. I wanted to post the code but it messed up everything...

Collapse
10: Response to SDM Changes (response to 1)
Posted by Roberto Mello on
I was just looking at your modifications and they look great Pascal. Being able to assign a patch to several bugs/features is great. As you noted, it'd be great to be able to bulk-close bafs as well. It's painful to close one by one when you are working on a series of bugs.

As for the projects belonging to a team, I already have that done for a couple of months. So once you finish your part and send me a patch, I can add it to my install and then we go from there.

Would you like to take over any of the other modifications we discussed here? Thanks a lot.

Collapse
11: Response to SDM Changes (response to 1)
Posted by Pascal Scheffers on
Okay, then I'll send you the code that I have now. The edit-a-patch fix only involves editing one-patch.tcl, which needs some additional cleanup anyway. I'll add that when the code has been merged again. I hope we didn't do any conflicting things.

I am willing to take on the other things. As I said, working on general-comments is a bit delayed as it requires acs-messaging, and the glossary needs GC... general-comments will take preference I think, as a lot of other modules use it.

Collapse
12: Response to SDM Changes (response to 1)
Posted by Roberto Mello on
Hmmm. I don't know. Bboard is not intended for software management, so I don't see how "bboard really works" applies here. But I do understand your points.

On 1.1, I don't think that'd be good. It's too much information that you don't need in the first page. What you need is by each module, a count of outstanding bugs and features. That is useful.

1.2 is already implemented, but it could be documented better, like what ArDigita did on their port of SDM. The button "I am interested" says "I am interested - Notify me of Changes" in their SDM.

1.3 is good.

1.4 would be really good on each modules' page. Again, I like what ArsDigita did on their SDM. They put a bar where you can choose which bafs to see: "outstanding, needinfo, resolved". We could change that to include something like "not patched".

On 2, if a bug is fixed, it means it has been fixed on CVS. The ArsDigita SDM has a date selection feature that we could also lfit the code from. That'd give you a way to see all the bugs that had been fixed since a certain date.

The double return issue on util_convert_plaintext_to_html is good. Great suggestions. Any more? Speak now or... send a patch later :)

Collapse
13: Response to SDM Changes (response to 1)
Posted by Roberto Mello on
Wow! Hadn't noticed this before: http://www.arsdigita.com/sdm/summary?package%5fid=8157. Now that's cool.

http://www.arsdigita.com/sdm/pvt/manage-incoming?package_id=8157 is where they can bulk-assign bafs to a developer. Also very nice.

Ben, if you don't like any of these things we are discussing, please chime in.

Collapse
14: Response to SDM Changes (response to 1)
Posted by Ben Adida on
I think there are a lot of good ideas being submitted. I'm happy to
see where these ideas take the SDM, and there's no need for
me to be a bottleneck on this one.

My only suggestion is to keep the system as simple as possible.
The SDM is something I originally hacked together very quickly.
The only reason it worked even this far is certainly not because
of any particularly smart or insightful idea: it's because it's quite
simple and straight-forward to use.

I'm looking forward to seeing these great ideas implemented!

Collapse
15: Response to SDM Changes (response to 1)
Posted by David Kuczek on
I just submitted the patch for break instead of paragraph.

If we get some of the mentioned functions up and running that would already be great.

We might want to give the person that submits a patch the option to select the filename where the bug is located in.
Collapse
16: Response to SDM Changes (response to 1)
Posted by Don Baccus on
Before *too* much more time has gone by, most of OpenACS 4 will have
been ported to PG and we'll be looking at other things to move to the
4.x framework.

So, Roberto and Pascal, guess how I think you should spend your summer
vacation :)

Seriously, the changes mentioned so far look pretty simple to
incorporate and we could certainly use them here at openacs.org as
well as in the 3.2.6 release.

But beyond one round of moderate effort at improving it, I think we
should be looking at an OpenACS 4 migration.  There are (count them)
three ticket tracker packages from aD, one based on workflow and
incomplete.  I happen to like the new-ticket-tracker in the SDM, but I
also think building the SDM pieces on top of acs-workflow makes a lot
of sense.

Anyway, I'm in no way trying to discourage effort to improve the SDM
in its current form - I want to get you two to think about
volunteering to help out (or take over, hey, I might as well swing for
the home run) the migration effort for 4.x.

There's also the project tracker package (alpha, development?  Haven't
checked it out yet) in aD's repository that I've not imported into our
OpenACS 4 CVS repository.  Project tracking is something not tackled
by the SDM.  Integration of code and/or ideas from the aD work along
with the nice, easy-to-use mechanics of the SDM could lead to
something exciting.

Something I wish I had now, actually, as I track OpenACS 4 project
progress via files in new-file-storage etc.

Collapse
17: Response to SDM Changes (response to 1)
Posted by Roberto Mello on
Don, your points are well taken. I'm not trying to do a major overhaul of the SDM, but since we are tracking the 3.2.x and 4.x trees on the current SDM and will be doing so for a while, we might as well make our lives easier (I have a personal interest in this since trying to match patches and closing bafs for 3.2.5 was not fun).

As you noted, the changes are simple, and it should only take a few hours to get them done and tested. Then, rock on 4.x ! :)

Collapse
18: Response to SDM Changes (response to 1)
Posted by Pascal Scheffers on
I agree. I wanted to point that out yesterday, but after a day to and from Paris, I didn't feel like it. Yes - lets do the things that take four hours a piece max and leave the rest for the 4.x version

I seriously like the idea of having a 4.x SDM that uses WF for project status et.al. CR for, well, content. Etc.

Collapse
Posted by Jonathan Marsden on
OK, I have a nifty new SDM package to look after.

I've been trying to create a release to correspond with my RPMs,
version 3.2.5-2.  It won't let me.  Is SDM assuming all version IDs
are of the form either A.B.C or possible A.B.CbD, where A,B,C, and D
are all integers?  It is starting to look that way.

If so, is removing this restriction likely to be simple?  Why not a
version string?

Thanks!

Collapse
20: Response to SDM Changes (response to 1)
Posted by David Kuczek on
I just posted two chat bugs and I would suggest the following features:

1.) On the summary page "open-bafs.tcl" you should see if a bug has been closed in the newest release of the software or not.

2.) open-bafs.tcl should also show the date when each bug has been submitted by a community-member.

3.) concerning the new version by Pascal: new-baf.tcl should have two textareas. One for the bug and parallely one for a patch that you might want to submit at the same time.