The following PLs/CGIs/PMs reference bug statuses in some way that is workflow-related: * ::Bug (is_open_state, check_can_change_field) * collectstats.pl (pull the list of statuses from the DB) * ::Constants (BUG_STATE_OPEN) * enter_bug.cgi (new bug creation) * importxml.pl (is the incoming bug's status valid for the product's workflow?) * post_bug.cgi (new bug creation) * process_bug.cgi (bug changing) * reports.cgi (change/remove the default-selected statuses) * show_bug.cgi (bug changing) There's also the following templates: * bug/knob.html.tmpl (which transitions are available to this bug) * list/edit-multiple.html.tmpl (which transitions are available to all bugs) Each of the files listed above will have its code moved as part of a bug that is marked as blocking this bug. There are certain files that do references status, but which I am not going to touch. There are also certain files that may be involved in workflows, but aren't involved in Stage 1. They are: * ::Bug: CheckIfVotedConfirmed will be handled in Stage 3 * editproduct.cgi: There is code that looks at UNCONFIRMED bugs, with regards to voting. That's covered in part 3. * editvalues.cgi: Doesn't allow for editing of statuses, yet. * query.cgi: This script calls Bugzilla::Field::get_legal_field_values to determine which values exist for Status, and since this sub goes to the bug_status table, which we're keeping, it's OK. * report.cgi: (see query.cgi) * votes.cgi: You might think that the "confirm bug if there are enough votes" code is here, but it's actually in Bug.pm. * whineatnews.pl: Considering the name, and the fact that I'd like this to go away, this file is going to be left unchanged. I've gone through each file in Bugzilla. If someone things I missed anything, let me know!

OK, here's what I'm looking at: A new module, Bugzilla::Workflow. This module contains most (if not all) of the operations required to determine what actions a person can take with a bug, not counting the basic editing of fields. So, for example, reassignment, resolution, and reopening of bugs would be actions whose availability would be controlled by Bugzilla::Workflow. If something wants to know what actions a user can perform, that something asks Bugzilla::Workflow, which might say "This user can reassign a bug to someone else, while keeping its current status unchanged." It's also possible that the module would say "This user can't do anything". That actually brings up an interesting note: A workflow (which, in my mind at least, encompasses everything handled by the "knobs" sitting just above the Commit button), while it does involve changes in status, is also responsible for determining if resolutions should be set, or if bugs are being reassigned. Although, at this stage of the game, there is only one workflow, and this one workflow can't be modified, I'd like to have somethings in place for when we move beyond this stage. In particular, there's the concept of a unique workflow ID (I'm assuming that workflow #1 is the default Bugzilla workflow), and object creation should require some sort of parameter, be it a workflow ID, the name/ID of a product (get an object representing the workflow the product is using), etc..

Okay, that's an approach that makes some sense in a functional world, but in an object-oriented world like we're moving toward, it would make more sense to have most things be contained in Bugzilla::Bug, and then to have a Bugzilla::Status object. In order to have customizable status, *all you need* is the statement "From this status, we can go to this status" expressed in the database.

I agree that we are moving into an object-oriented world, but I disagree that the functionality should be placed at the domain objects. The domain objects should contain data and nothing further. All kind of business logic should be handled in a separated business layer, which can be divided into modules such as Bugzilla::Workflow etc. (as suggested in #1). The main purpose for the domain objects are to create a model, which should be separated from the business logic. In this way it will be easier to change either the domain model or business logic without any mayor side-effects on both. During the design the data access layer fetches data from the database and returns the data to the upper-level layer as the domain model prescribes. The business layer operates at this domain model (where some objects are returned from the data access layer others are newly created by factories) when it (the business layer) is implementing the business rules.

No, Bugzilla isn't architected in some sort of MVC pattern. There's absolutely no reason to make the objects data-only. None of them are now, and none of them will be in the future. There is no logic I can imagine that can't be expressed inside of Bugzilla::Bug and Bugzilla::Status. Workflow is all status, basically. Any workflow that's not status should be handled in a separate bug outside of bug 101179. Otherwise we're doing two things in one bug.

(In reply to comment #4) > No, Bugzilla isn't architected in some sort of MVC pattern. There's absolutely > no reason to make the objects data-only. None of them are now, and none of them > will be in the future. I agree with that point. If there were modules that primarily contained data, with little associated logic, that's increasing becoming not the case. We seem to be moving much more towards centralizing logic of all sorts into libraries, with data kept in the database. This allows us to have things other than the CGIs doing things in Bugzilla, without having to duplicate code too much. > There is no logic I can imagine that can't be expressed inside of Bugzilla::Bug > and Bugzilla::Status. Workflow is all status, basically. Any workflow that's > not status should be handled in a separate bug outside of bug 101179. Otherwise > we're doing two things in one bug. > I have never thought of bug 101179 as being only related to status changes: Although status changes are the bulk of the issue, there are two additional things which are intimately tied to status changes: A status change could involve setting or clearing a resolution. Also, a change in status could take place as part of a change in assignee. There is also the matter of setting the everconfirmed attribute on a bug. Although the summary of bug 101179 only mentioned statuses, there are 7 bugs which are marked as a duplicate of bug 101179. Four of those 7 bugs mention workflow. On of those four bugs (bug 333096) actually asks for a much more detailed form of workflow (assigning bugs based on priority, and automatic escalation). I admit, I'm not trying to go for the goals that are mentioned in bug 333095, but I am trying to get the base of such a system laid. That to me involves more than just status changes. It is bigger, yes, but that's why I'm trying to break it into smaller pieces. In the last few days, I've been working up a POD listing the various operations that I'd put into a Bugzilla::Workflow module. I'll post it, and we can see if it helps justify a single module to cover all of this.

Okay. How about first we make a patch that just deals with statuses, and then we do the other workflow details? And certain statuses can just remain "special" for now and require a resolution, like RESOLVED, VERIFIED, and FIXED.

OK, the first viewable version of the POD for Bugzilla::Workflow. It was produced using pod2html, so some of the links don't work and/or look weird. This is (for the most part) what I'm looking at as the functionality for workflows. Initially, almost everything will be hard-coded (or work off of hard-coded rules), eventually becoming database-driven at the end.

Looks like Firefox did some strange selecting of the group radio boxes on the mass-change page when I clicked refresh in my browser before that last mass-change. This is not really a security bug. :-)

I agree to have workflow stuff in Bugzilla::Workflow, including statuses. I also agree that in order to keep things small, you should start with statuses only, ignoring everything else for now. As soon as this code is checked in, it should be easier to do the remaining tasks. See how big my patch was for customized resolutions, and this one was easier to fix. So imagine how big the one for statuses will be, knowing that it's less obvious to fix. We freeze in one month, it's high time to attach your patches for review, knowing that a review may need several weeks.

This bug is retargetted to Bugzilla 3.2 for one of the following reasons: - it has no assignee (except the default one) - we don't expect someone to fix it in the next two weeks (i.e. before we freeze the trunk to prepare Bugzilla 3.0 RC1) - it's not a blocker If you are working on this bug and you think you will be able to submit a patch in the next two weeks, retarget this bug to 3.0. If this bug is something you would like to see implemented in 3.0 but you are not a developer or you don't think you will be able to fix this bug yourself in the next two weeks, please *do not* retarget this bug. If you think this bug should absolutely be fixed before we release 3.0, either ask on IRC or use the "blocking3.0 flag".

> In order to have customizable status, *all you need* is the statement "From > this status, we can go to this status" expressed in the database. I agree with others regarding this, Max. I think that workflow customizations should be flexible enough to be able to attach certain status changes to fields being set or cleared. We already have this coded (as mentioned above) against resolution. On the other hand, I see a number of cases where it's very appropriate to similarly attach status to other customized fields (i.e. customized status DOCUMENTED requires a DocumentationURI to be set). I don't know that it's fair to require that this be implemented in 3.2, but I think we ought to at least think in that direction during the development process. :-) Kevin

(In reply to comment #11) > > In order to have customizable status, *all you need* is the statement "From > > this status, we can go to this status" expressed in the database. > > I agree with others regarding this, Max. [snip] Yes, but THIS BUG should only deal with STATUS CHANGES. Nobody's arguing that Bugzilla shouldn't advance at some point in the future to be more customizeable. Basically, what's being said here is "I want fields to be able to link to each other and update each other!" Great! We don't have that feature in Bugzilla AT ALL yet. So file it in some other bug. THIS bug is about having a customizeable status field, and thus it should do ONLY THAT. A common mistake that people make is designing too far into the future. The right way to go about development, particularly in an open source project, is to design for the specific requirement (singular) that you *know* is broadly needed, and implement them on top of the system you already have. If that can't be done, or can't be done with *simple* code, then you do re-architecture. Rinse and repeat. Violations of that principle are what lead to crazy, messy code that nobody wants to maintain after a few years. People design massive changes that do everything in some extremely specific way, based on a nearsighted design. (I'm not accusing you of this or any of what I've said above, Kevin, I'm just using this space to explain something that I've come across often.) Then, much later when somebody comes up with some feature that you *never thought of* but would be *extremely useful*, you *can't implement it*, because you've got this unwieldy very-specific design that handled only what you needed at the time. It's fine to design for what you need at the time, as long as it's a *small design*. So, I just make all designs into small designs, and we're fine. That's why I'm so adamant about "one bug, one issue." Because it encourages small designs, and it keeps Bugzilla maintainable. That said, the small design that can be focused on right now is customizeable statuses, and what we would minimally need to do to get there.

This is the very last patch about bug status workflow. As soon as it's checked in, all we have to do is to enable bug status edition from editvalues.cgi.

Comment on attachment 266226 [details] [diff] [review] patch, v1 No comments :-) r=gerv. Gerv

Checking in checksetup.pl; /cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v <-- checksetup.pl new revision: 1.553; previous revision: 1.552 done Checking in sanitycheck.cgi; /cvsroot/mozilla/webtools/bugzilla/sanitycheck.cgi,v <-- sanitycheck.cgi new revision: 1.131; previous revision: 1.130 done Checking in Bugzilla/Bug.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v <-- Bug.pm new revision: 1.183; previous revision: 1.182 done Checking in Bugzilla/Config.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Config.pm,v <-- Config.pm new revision: 1.71; previous revision: 1.70 done Checking in Bugzilla/Config/BugChange.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Config/BugChange.pm,v <-- BugChange.pm new revision: 1.3; previous revision: 1.2 done Checking in Bugzilla/Install/DB.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Install/DB.pm,v <-- DB.pm new revision: 1.35; previous revision: 1.34 done Checking in template/en/default/admin/admin.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/admin.html.tmpl,v <-- admin.html.tmpl new revision: 1.2; previous revision: 1.1 done Checking in template/en/default/admin/params/bugchange.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/params/bugchange.html.tmpl,v <-- bugchange.html.tmpl new revision: 1.4; previous revision: 1.3 done