Persisting/Saving with broken rules?

hurcane replied on Tuesday, June 13, 2006

I remember an earlier discussion around this topic, but I can't find it. Perhaps it was on the old forum.

ValidationRules are intended to control whether an editable object can be saved. If your use case says the user can save the object with a blank field, then the field is not required for that use case.

You must have some other use case at which point the field is required. That's where the business rule belongs. Not all business rules are validation rules.

I have a Quote object for which the customer ID is optional. To convert the quote into an order, the customer ID must be filled in (that is, the accounting department has run a credit check and assigned a customer ID for the prospect). That rule is enforced by the QuoteConvert command object.

Fireworks replied on Wednesday, June 14, 2006

I should also mention that I actually need to have the broken rules for display purposes.

So really what I'm looking for is a way to run all of the broken rules, show the errors, and still allow a save as long as the essential fields of the object have been set. Lets call this the 'temporary' state. Data in db but the object isn't finished.

Later, I want to save the object but, this time I want all rules to have been met, before a save can occur. Lets call this the 'permanent' state.

At any point I need to be able to show what broken rules there are, so it really isn't so much about running the rules or not, but more about 'enforcing' some rules all the time, and other rules some of the time.

I was thinking about some how adding a 'severity' to the object that the save would check and enforcment would be determined on that. Not sure how though...

esteban404 replied on Wednesday, June 14, 2006

Where this was a "show stopping" requirement, the users move a process from one phase to the next by initiating a "promote" event which sets a bool promote flag in the object. This flag is part of the business rule along with the phase. Once promoted, the flag is off and they continue happily until they need to promote it again.

I know it's not good to do this, but it does help and the object remains valid, but incomplete by phase standard.

xal replied on Wednesday, June 14, 2006

Hey all!
I implemented the severity for rules in a way that is backwards compatible and easily handled.
The change is very simple and involves only changes to the validation rules namespace.
I created an enum that may have more possible values, but for simlpicity's sake:

Namespace Validation
Public Enum SeverityLevel
    Broken = 0
    Warning = 1
End Enum
End Namespace

Added This to RuleArgs:
Private mSeverity As SeverityLevel = SeverityLevel.Broken
Public Property Severity() As SeverityLevel

Note that it defaults to Broken so that it doesn't break existing code. Also, it is the most reasonable default.

This to BrokenRule:
Private mSeverity As SeverityLevel
Public ReadOnly Property Severity() As SeverityLevel

Changed ValidationRules.IsValid to:

Friend ReadOnly Property IsValid() As Boolean
Get
   For Each br As BrokenRule In BrokenRulesList
   If br.Severity = SeverityLevel.Broken Then
      Return False
   End If
   Next
   Return True
End Get
End Property

This makes it very easy to have different levels of rules and you handle it inside your rule methods.

The only thing that needs a more profound debate is what should be done with the IDataErrorInfo interface.
Should we not return warnings? Or should warnings be returned prepending a "Warning: " text?
Personally I vote for not returning them because doing so would bring the state of the form to invalid (when calling Validate / ValidateChildren).

Rocky, are you around? What do you think?

Andrés

xal replied on Wednesday, June 14, 2006

Andy, you only need to handle the logic in your rule. It's as easy as:

Private Shared Function ValidateLastName(ByVal target As ObjectType, _
        ByVal e As Csla.Validation.RuleArgs) As Boolean

    If target.mLastName.Trim().Equals(String.Empty) Then
    If target.State = States.AllowInvalid Then
        'By default severity is broken
        e.Severity = Csla.Validation.SeverityLevel.Warning
    End If
      e.Description = "Last Name cannot be empty"
      Return False
    End If
    Return True
End Function

I didn't create another collection because:
They're still rules, and they are broken. It's the severity that changed, and I don't see how that smells.
Adding another collection adds more complexity.
What happens if you add another severity? Are you going to add yet another collection?

Anyway, that's just the way I think.

Also, to make things more complete we should also implement a HasWarnings() property, so that we can show the never aging "Are you sure you want to save this?".

Andrés

odie replied on Friday, June 16, 2006

Thought i'd put my 2 cents worth in on something I just tried out that might be of help, though it does required some modifications to the CSLA rules classes (not ideal.)

I did a quick test and it works as indicated below.

Here's what changes I made:
1. added an optional ErrorSavable as boolean parameter to AddRule which is defaulted to false, along with other interal changes to pass this into the BrokenRule class.
2. added ErrorsSavable property to the BrokenRulesCollection that can be set within your BO

Now when I add my rules I can indicate which ones are savable and which are not depending on what ErrorsSavable is set to. If it is set to true then IsValid returns true dispite the broken rules collection containing errors that are tagged as ErrorSavable = true. This way you can display broken rules but still allow the object to be saved.

When some condition is met where you require ALL rules to be satisifed you simply set ErrorsSavable = false in the BrokeRulesCollection and IsValid will now return false if any errors exist in the collection just as it currently does.

Any thoughts would be appreciated.

hurcane replied on Wednesday, June 14, 2006

I like xal's idea of having a severity to distinguish rules that prevent save and rules that are more informational. However, it does seem to have a barely discernible code smell. I wonder if the best solution would be to have a parallel structure of WarningRules?

I thought I would throw out another idea that might work without changing the framework. This is kind of smelly, too, but sometimes you just go with what works. I haven't tested this idea, so it may not work at all. Nevertheless...

If your rule handler is internal to the object, you could include a state variable to indicate that the rule is to be ignored. The rule handler logic would check the state variable and return True when the rule is to be ignored.

Override the Save() method. In the Save method, do this...

m_IgnoreRuleA = True
CheckRules()
mybase.Save
m_IgnoreRuleA = False
CheckRules()

The overridden Save method temporarily "turns off" the rule, which allows the base Save method to be successful using the otherwise standard logic.

If your rule handler is external, you can use a custom RuleArgs to include the state variable.

The more I think about it, the more my nose is crinkling.

JoeFallon1 replied on Wednesday, June 14, 2006

One idea is to have 2 BOs and 2 DB tables.

The final data is stored in Table1.

Table2 is where you store incomplete data.

Provide 2 buttons:

Save

Save To Hold.

If they can't Save, then they can just Save to Hold which persists the data in Table2.

Instantiate a SaveToHold BO and then set its properties from the regular BO.

Since its rules are different (or non-existant) you can save to Table2.

Joe

Fireworks replied on Wednesday, June 14, 2006

I don't think two tables is required. To me the problem is behavioral not data.

Saying that, in this case two objects isn't going to do it either. I need all of the rules and their messages. That's the thing. Some data is essential to a save (ie fk's) but other data isn't but may be required to fulfill business requirements (ie. last name is mandatory).

Basically, right now, if you have any broken rules, you can't save. Which is fine in a lot of scenario's, but when you need to do a 'partial' save of the data, and still provide feedback to the GUI as to what is missing there isn't any way to do it. (that I see anyways)

At the moment it would seem that a combination of using different internal AddBrokenRules delegates (this would allow the object to be init'd in context to requirements, like having an Address object that has different required fields based on owner), and using the above mentioned changes for adding a 'severity' property to the Rule itself, and then checking for a specific severity when the value for IsValid is being decided.

This would allow what I want, increasing or decreasing the enforcment of required fields to determine a valid object, and what fields are actually doing to be checked and with what rules.

I don't suppose anybody has this code, oh, just lying around....?

xal replied on Wednesday, June 14, 2006

Yes, as I said I have done and tested this. It works as expected but there are still things that need some work. Primarily, Rocky's acceptance. Also, we should define whether IDataErrorInfo should return warnings or not (for now I vote not, but I haven't completely made my mind). Maybe even exposing something like HasWarnings and also if there's a need for a third (or more) severity levels and what to do for each. Maybe an intermediate one that allows saving but is returned by IDataErrorInfo.
Anyway, what's most important is that theres a severity level that enforces rules on save and one that doesn't.

Andrés

RockfordLhotka replied on Tuesday, July 25, 2006

I'd like to resurrect this thread briefly, as I'm actually working on the concept of rule severity for 2.1.

Where I'm going with this, is that you'll set the severity inside your rule method, along with the description:

If <rule is broken> Then
e.Description = "You did something bad!"
e.Severity = RuleSeverity.Error
Return False

That part seems quite clear to me (other than deciding how many severities are needed - but Error and Warning seem sufficient in my mind).

The part where I'm undecided is on how to expose the broken rules (with different severities) outside the object.

Andrés took the approach of just returning them from BrokenRules like any other rule. And this is OK, though it does mean IsValid goes from a trivial operation (returning .Count) to something more expensive - and I dislike that because it gets called quite often - but I also think I can optimize this behavior.

The unknown here is whether warnings should appear in IDataErrorInfo - but probably not, as that would mess up form-level validation concepts.

The other idea on the table is to have BrokenRules and WarningRules as different collections. Only BrokenRules would affect IsValid and IDataErrorInfo - you'd have to go yourself to get items out of WarningRules.

Regardless of the joint collection or seperate collection, I'm considering whether an IDataWarningInfo interface is required. I know data binding wouldn't use it, but using such an interface you could create a WarningProvider extender control like ErrorProvider and that would be cool.

It seems to me that even if I do go with a joint BrokenRules collection (which is the option I tend to favor), that this enhanced BrokenRules will need a way so you can easily just get the error or just get the warning items. Perhaps the new FilteredBindingList is the solution there though...

xal replied on Tuesday, July 25, 2006

Hi Rocky!

I'd say we there should be another level: Information. That way you can pass information to the user that is really not that critical for an error or even a warning.

For example:
You buy a product and pay with a credit card. The payment object could have a rule that tells you in which month the actual payment will be debited from your account: "The credit card operation will be processed in august"

About processing to know how many rules of each kind you have, you could override the collection's InsertItem and then do something like:

    Protected Overrides Sub InsertItem(ByVal index As Integer, ByVal item As BrokenRule)
        MyBase.InsertItem(index, item)
        If item.Severity = RuleSeverity.Error Then
           mErrorCount += 1
        End If
    End Sub

And then on RemoveItem you do the same, but substracting 1. That way you can keep a count of how many rules of each type you have.

<sidenote>
This is an approach I've taken to validate rules that involve looking for items of a same type (type as in item.CustomerType = "Compulsive buyer") in a collection. It costs nothing to maintain that variable inside the collection and rules don't need to iterate through a collection to find out how many items have a property that equals some value
</sidenote>

I think it's definitely easier to maintain and to use just one collection of BrokenRules. Absolutely, you can have a filtered list that returns just Errors or Warnings or whatever...

I've already thought of having and extended error provider and I will contribute that as soon as I have it. I already have a treeview that displays the rules in different colors and with different icons according to severity. I plan to put both in cslacontrib when Rocky finishes implementing everything.

Andrés

Igor replied on Tuesday, July 25, 2006

I think that support for rules that may be broken when we save a BO is only one of a few similar things: another matter to consider might be support for multiple saving operations, e.g. (saving Invoice Quota) and (conversion Invoice Quota to Invoice). We may have two buttons on the Invoice/Quota form, say Save and Convert, bound to two properties, say IsSavable and IsConvertable. We can use two different collections of business rules to support these properties: SavingRules and ConversionRules. If we try to build support for such scenarios into the framework, we would probably end up with a collection of collections of broken rules … not sure that such a complication would be reasonable, but maybe it would ...

Igor

RockfordLhotka replied on Tuesday, July 25, 2006

Igor:

I think that support for rules that may be broken when we save a BO is only one of a few similar things: another matter to consider might be support for multiple saving operations, e.g. (saving Invoice Quota) and (conversion Invoice Quota to Invoice). We may have two buttons on the Invoice/Quota form, say Save and Convert, bound to two properties, say IsSavable and IsConvertable. We can use two different collections of business rules to support these properties: SavingRules and ConversionRules. If we try to build support for such scenarios into the framework, we would probably end up with a collection of collections of broken rules … not sure that such a complication would be reasonable, but maybe it would ...

Igor

I'd suggest that what you are describing sounds like it is either two use cases (with their own objects), or is a case where you should have other objects helping you out. I certainly don't plan on allowing you to set up different sets of rule methods per-class - that'd be nuts!!

Igor replied on Tuesday, July 25, 2006

With all due respect,

1. It works fine. And no maintainability problems.

2. The alternatives are: maintaining two collections of broken rules, or maintaining two BOs (and possibly more: the two BOs would have many identical members). Actually, I oversimplified my current situation: the BO has to support more than 2 persistence operations, so the trade-off is between one BO with a few sets of business rules and a whole set/hierarchy of objects (the number of the objects will be greater than the number of sets of rules).

Igor replied on Wednesday, July 26, 2006

Andres,

you wrote: "Your IsConvertable property could loop through the broken rules collection and see if there are any that affect convertibility... "

You are right, but the LOOPING is a more PROCEDURAL method compared with the two collections of rules.

BTW, I did not want to provoke another anti-datacentric discussion, but Rocky did consider having multiple collections of rules in an object, so I decided to point to another (different, but similar, IMO) scenario where multiple collections might be useful. No, I am not sure that we really need them in CSLA base classes, and I am quite happy to support multiple persistence operations in my concrete BOs. Now I am thinking about this matter again ... and I am not sure at all that having a separate class to support each persistence operation (= each button) is ALWAYS a good idea. Personally, I would weight the pros and contras on per case basis; and the criterion would be: Maintainability.

Igor

xal replied on Wednesday, July 26, 2006

Igor,
As I said, you don't need to actually loop throught the collection, you can keep track of the broken rules that affect your "convertibility" by listening to the broken rules collection's events.

That said, come oooon... you can't seriously say those 3 lines of code, maybe 4 is procedural. Looping is a common practice and object oriented is not meant to replace such a basic and indispensable operation, it is meant to aid it and make it more maintainable (amongst other things, of course).
Anyway, you could also have a view that shows only the rules you're interested in, but ultimately, that will loop through the rules to find the ones you're interested in.

<joke>
Even if you had two collections, rocky's going to have to do the procedural loop through the validation rules to run the validations and fill one broken rules collection or the other. Of course, for that he'll have to use the conditional IF which dates since before the assembly times.
</joke>
Wink [;)]

Andrés

odie replied on Friday, July 28, 2006

I just had a thought on this issue, though I could be completely out to lunch on it, but would like your opinion.

Why not simply expand the current brokenrules as follows:

What if you added two more parameters related to two additional properties in your object. One telling under what condition to generate the error and what condition to generate the warning. Each would simply return a boolean value when a validation rules are processed.

ValidationRules.AddRule(ByVal handler as csla.validation.ruleHandler, ByVal propertyname as string, ByVal ErrorConditionPropertyName as string, ByVal WarningConditionPropertyName as string)

Then each validation rule generates the errors and warnings based on the two additional parameters and has the ability to convert a warning into an error internally. This way when the state of your BO changes a warning would automatically be converted to an error as required when a given property changes.

Simple Eg.

Quote object that requires an estimate document number to be submitted to a client but not to save. Additionally the quote requires a PO number to be converted to an Invoice. (not the greatest example but all I could come up with on short notice)

ValidationRule.AddRule(AddressOf CommonRules.StringRequired, "EstDocNumber", "IsSubmitToClient", "IsNotSubmitToClient")

ValidationRule.AddRule(AddressOf CommonRules.StringRequired, "PONumber", "IsInvoice")

You would then have two readonly properties IsSubmittedToClient and IsNotSubmittedToClient

The above lines would thus generate a warning message when EstDocNumber is not specified and when then status is "not submitted" to the client but the user could still save. As soon as the status is changed to "submitted to client" the warning would be converted to an error and thus prevent the object from being save.

The second example would only generate an error (no warning) if you try to convert the quote to an invoice without first specifying a PO number.

You might also have to add a list of what additional properties the validation rule needs to listen to to otherwise it will only get evaluated when the main property changes

I hope this makes sense?

RockfordLhotka replied on Monday, July 31, 2006

odie:

Then each validation rule generates the errors and warnings based on the two additional parameters and has the ability to convert a warning into an error internally. This way when the state of your BO changes a warning would automatically be converted to an error as required when a given property changes.

I think you'll be able to do this with the new approach in 2.1, though not quite as you describe.

You can create rule methods to do anything you want in response to the new property value and the current state of the object. So your rule method could check to see if the object is an invoice, or has some specific value set, and make another field required or not accordingly. You can do this today.

With the 2.1 changes, your custom rule method will be able to intelligently decide whether the rule "violation" should be reflected as an Error or a Warning by setting e.Severity.

The code in BusinessBase.Save() won't change from today - it won't allow you to save an object when IsValid is false. If you don't like that behavior you can override Save() just like you can today. Then you could, if you wanted, prevent saving when there are warnings or whatever you choose.

RockfordLhotka replied on Wednesday, July 26, 2006

I think it depends on how much work is done to do a "convert".

You have a Quote and you have an Order - that makes sense. Converting one to the other is a process, and so there's the need for a "quote to order converter".

If you have Order.NewOrder(quote), then your coverter is that factory method - which is pretty clear.

But you could also have a "order = QuoteConverter(quote)" which would be even more clear. And more importantly, would consolidate the convert process code into its own logical and isolated place.

Yes, the factory method is also such a place, but it is buried within the Order class and so the discoverability of the code is less than if it were in its own class. But the drawback is that the consuming developer needs to know about yet another object...

So what about the best of both worlds? Use the factory approach, but inside the factory you call QuoteConverter. So the factory is just one line of code and the complexity is nicely encpasulated in a logical home. QuoteConverter would be Friend/internal so the UI developer would have the simplest interface possible.

Michael Hildner replied on Tuesday, July 25, 2006

FWIW, I used to use a commercial business object framework, called Mere Mortals. I was using this with .NET 1.1.

In that framework, the author opted to have a BrokenRules collection and a BrokenWarnings collection (might not have the names right).

If you had a broken rule, you could not save the object, if you had a broken warning, you could save. That framework had methods like "string GetBrokenWarnings()" so you could display a message box with the broken warnings/rules.

I like the idea of an IDataWarningInfo interface. I think that would make for a great UI experience.

Mike

Igor replied on Wednesday, June 14, 2006

Fireworks,

you asked: "Is there a way/suggestion to persist data to the database even though the object is technically invalid?"

I am doing that all the time. Simply by calling Update. To let DP_Update know which saving operation is actually requested I use a class-level flag of some sort. See some further detail here: http://forums.lhotka.net/forums/post/1025.aspx

Igor

Persisting/Saving with broken rules?

Fireworks posted on Tuesday, June 13, 2006

xal replied on Tuesday, June 13, 2006

ajj3085 replied on Tuesday, June 13, 2006

hurcane replied on Tuesday, June 13, 2006

Fireworks replied on Wednesday, June 14, 2006

esteban404 replied on Wednesday, June 14, 2006

xal replied on Wednesday, June 14, 2006

ajj3085 replied on Wednesday, June 14, 2006

xal replied on Wednesday, June 14, 2006

esteban404 replied on Thursday, June 15, 2006

Igor replied on Thursday, June 15, 2006

odie replied on Friday, June 16, 2006

hurcane replied on Wednesday, June 14, 2006

Fireworks replied on Wednesday, June 14, 2006

JoeFallon1 replied on Wednesday, June 14, 2006

ajj3085 replied on Wednesday, June 14, 2006

xal replied on Wednesday, June 14, 2006

Fireworks replied on Wednesday, June 14, 2006

xal replied on Wednesday, June 14, 2006

RockfordLhotka replied on Tuesday, July 25, 2006

xal replied on Tuesday, July 25, 2006

Igor replied on Tuesday, July 25, 2006

RockfordLhotka replied on Tuesday, July 25, 2006

Igor replied on Tuesday, July 25, 2006

xal replied on Tuesday, July 25, 2006

Igor replied on Wednesday, July 26, 2006

xal replied on Wednesday, July 26, 2006

odie replied on Friday, July 28, 2006

RockfordLhotka replied on Monday, July 31, 2006

ajj3085 replied on Wednesday, July 26, 2006

RockfordLhotka replied on Wednesday, July 26, 2006

Michael Hildner replied on Tuesday, July 25, 2006

david.wendelken replied on Wednesday, June 14, 2006

xal replied on Wednesday, June 14, 2006

Igor replied on Wednesday, June 14, 2006