void and retire reasons in 1.5

[Michael Seaton]

Some javascript/style in this post has been disabled (why?)

Hi all,

Today, I ran into 2 different but related issues I want to bring up.  As of 1.5, we have now centralized the logic for "retiring" and "voiding" our objects via the BaseVoidHandler and RetireSaveHandler.  Both of these Handlers require that a "reason" be supplied in order to either void or retire an object, and if no reason is provided, an APIException is thrown.

In general, my question is whether we really want to enforce this at the API level?  I have seen many instances where people have just hacked around this by either passing a space or some fixed generic string to get around this restriction, and where this hasn't been done, I have occasionally seen pages that once worked in 1.4 throwing stacktraces (i.e. voiding Patient Programs on the Patient Dashboard).  The fact that people are coding around this seems to indicate to some degree that we should rethink it.

To be specific, here are the 2 things I experienced today (on 1.5.x build) that brought this to my attention:

1. It is currently impossible to delete a patient program from the patient dashboard, since there is no way to pass in a reason.  Presumably this worked in 1.4 and with the new changes in 1.5 things have become stricter.  Although I think we actually _should_ change the UI to provide a way to enter a reason, I'm not 100% sure this should be required.

2. I tried to add a retired Form to as a property on another object, and then save the other object.  This caused a RuntimeException due to the fact that no retire reason was supplied on the Form - even though I had not changed this object at all.  This also seems to be a side effect of previous versions allowing the reason to be null, which is now causing problems due to tighter restrictions in the current code.

I'd be interested in others thoughts as to whether we should loosen this restriction on voidable, retireable or both.

Thanks,
Mike

---

[hidden email] from OpenMRS Developers' mailing list

[Burke Mamlin]

Some javascript/style in this post has been disabled (why?)

Why are people putting a space or hacking around the requirement instead of putting in a canned reason -- i.e., "Deleted from dashboard" or "Auto-deleted by foo processor"?  The reason doesn't *have* to be manually entered if it's either implied or unnecessary for the particular workflow; a canned reason is more helpful than nothing at all.  Perhaps it's a training/documentation problem.

-Burke

On Nov 4, 2009, at 7:41 PM, Michael Seaton wrote:

Hi all,

Today, I ran into 2 different but related issues I want to bring up.  As of 1.5, we have now centralized the logic for "retiring" and "voiding" our objects via the BaseVoidHandler and RetireSaveHandler.  Both of these Handlers require that a "reason" be supplied in order to either void or retire an object, and if no reason is provided, an APIException is thrown.

In general, my question is whether we really want to enforce this at the API level?  I have seen many instances where people have just hacked around this by either passing a space or some fixed generic string to get around this restriction, and where this hasn't been done, I have occasionally seen pages that once worked in 1.4 throwing stacktraces (i.e. voiding Patient Programs on the Patient Dashboard).  The fact that people are coding around this seems to indicate to some degree that we should rethink it.

To be specific, here are the 2 things I experienced today (on 1.5.x build) that brought this to my attention:

1. It is currently impossible to delete a patient program from the patient dashboard, since there is no way to pass in a reason.  Presumably this worked in 1.4 and with the new changes in 1.5 things have become stricter.  Although I think we actually _should_ change the UI to provide a way to enter a reason, I'm not 100% sure this should be required.

2. I tried to add a retired Form to as a property on another object, and then save the other object.  This caused a RuntimeException due to the fact that no retire reason was supplied on the Form - even though I had not changed this object at all.  This also seems to be a side effect of previous versions allowing the reason to be null, which is now causing problems due to tighter restrictions in the current code.

I'd be interested in others thoughts as to whether we should loosen this restriction on voidable, retireable or both.

Thanks,
Mike

---

[hidden email] from OpenMRS Developers' mailing list

[Michael Seaton]

Good point - I think documentation/training on this would certainly help developers to utilize this better.

Burke, I take it from your response that you think the API should enforce this be non-null for all retired and voided objects?



-----Original Message-----
From: [hidden email] on behalf of Burke Mamlin
Sent: Wed 11/4/2009 8:25 PM
To: [hidden email]
Subject: Re: [OPENMRS-DEV] void and retire reasons in 1.5
 
Why are people putting a space or hacking around the requirement  
instead of putting in a canned reason -- i.e., "Deleted from  
dashboard" or "Auto-deleted by foo processor"?  The reason doesn't  
*have* to be manually entered if it's either implied or unnecessary  
for the particular workflow; a canned reason is more helpful than  
nothing at all.  Perhaps it's a training/documentation problem.

-Burke

On Nov 4, 2009, at 7:41 PM, Michael Seaton wrote:

> Hi all,
>
> Today, I ran into 2 different but related issues I want to bring  
> up.  As of 1.5, we have now centralized the logic for "retiring" and  
> "voiding" our objects via the BaseVoidHandler and  
> RetireSaveHandler.  Both of these Handlers require that a "reason"  
> be supplied in order to either void or retire an object, and if no  
> reason is provided, an APIException is thrown.
>
> In general, my question is whether we really want to enforce this at  
> the API level?  I have seen many instances where people have just  
> hacked around this by either passing a space or some fixed generic  
> string to get around this restriction, and where this hasn't been  
> done, I have occasionally seen pages that once worked in 1.4  
> throwing stacktraces (i.e. voiding Patient Programs on the Patient  
> Dashboard).  The fact that people are coding around this seems to  
> indicate to some degree that we should rethink it.
>
> To be specific, here are the 2 things I experienced today (on 1.5.x  
> build) that brought this to my attention:
>
> 1. It is currently impossible to delete a patient program from the  
> patient dashboard, since there is no way to pass in a reason.  
> Presumably this worked in 1.4 and with the new changes in 1.5 things  
> have become stricter.  Although I think we actually _should_ change  
> the UI to provide a way to enter a reason, I'm not 100% sure this  
> should be required.
>
> 2. I tried to add a retired Form to as a property on another object,  
> and then save the other object.  This caused a RuntimeException due  
> to the fact that no retire reason was supplied on the Form - even  
> though I had not changed this object at all.  This also seems to be  
> a side effect of previous versions allowing the reason to be null,  
> which is now causing problems due to tighter restrictions in the  
> current code.
>
> I'd be interested in others thoughts as to whether we should loosen  
> this restriction on voidable, retireable or both.
>
> Thanks,
> Mike

_________________________________________

To unsubscribe from OpenMRS Developers' mailing list, send an e-mail to [hidden email] with "SIGNOFF openmrs-devel-l" in the  body (not the subject) of your e-mail.

[mailto:[hidden email]?body=SIGNOFF%20openmrs-devel-l]


_________________________________________

To unsubscribe from OpenMRS Developers' mailing list, send an e-mail to [hidden email] with "SIGNOFF openmrs-devel-l" in the  body (not the subject) of your e-mail.

[mailto:[hidden email]?body=SIGNOFF%20openmrs-devel-l]

[Burke Mamlin]

Some javascript/style in this post has been disabled (why?)

Actually, now I would favor something like...

/**

 * ...

 *   reason - reason foo is being deleted/voided.  If you are 

 *      automatically voiding/deleting data or working without user input, 

 *      then supply a canned reason that explains your process 

 *      -- e.g., "Auto-deleted by foo processor."

 * ...

 */

function ... {

...

if (reason == null || reason.trim().length() == 0)

throw APIException("Reason for voiding/deleting data is required.");

...

}

:-)

-Burke 

On Nov 4, 2009, at 9:00 PM, Michael Seaton wrote:

Good point - I think documentation/training on this would certainly help developers to utilize this better.

Burke, I take it from your response that you think the API should enforce this be non-null for all retired and voided objects?



-----Original Message-----
From: [hidden email] on behalf of Burke Mamlin
Sent: Wed 11/4/2009 8:25 PM
To: [hidden email]
Subject: Re: [OPENMRS-DEV] void and retire reasons in 1.5

Why are people putting a space or hacking around the requirement  
instead of putting in a canned reason -- i.e., "Deleted from  
dashboard" or "Auto-deleted by foo processor"?  The reason doesn't  
*have* to be manually entered if it's either implied or unnecessary  
for the particular workflow; a canned reason is more helpful than  
nothing at all.  Perhaps it's a training/documentation problem.

-Burke

On Nov 4, 2009, at 7:41 PM, Michael Seaton wrote:

Hi all,

Today, I ran into 2 different but related issues I want to bring  

up.  As of 1.5, we have now centralized the logic for "retiring" and  

"voiding" our objects via the BaseVoidHandler and  

RetireSaveHandler.  Both of these Handlers require that a "reason"  

be supplied in order to either void or retire an object, and if no  

reason is provided, an APIException is thrown.

In general, my question is whether we really want to enforce this at  

the API level?  I have seen many instances where people have just  

hacked around this by either passing a space or some fixed generic  

string to get around this restriction, and where this hasn't been  

done, I have occasionally seen pages that once worked in 1.4  

throwing stacktraces (i.e. voiding Patient Programs on the Patient  

Dashboard).  The fact that people are coding around this seems to  

indicate to some degree that we should rethink it.

To be specific, here are the 2 things I experienced today (on 1.5.x  

build) that brought this to my attention:

1. It is currently impossible to delete a patient program from the  

patient dashboard, since there is no way to pass in a reason.   

Presumably this worked in 1.4 and with the new changes in 1.5 things  

have become stricter.  Although I think we actually _should_ change  

the UI to provide a way to enter a reason, I'm not 100% sure this  

should be required.

2. I tried to add a retired Form to as a property on another object,  

and then save the other object.  This caused a RuntimeException due  

to the fact that no retire reason was supplied on the Form - even  

though I had not changed this object at all.  This also seems to be  

a side effect of previous versions allowing the reason to be null,  

which is now causing problems due to tighter restrictions in the  

current code.

I'd be interested in others thoughts as to whether we should loosen  

this restriction on voidable, retireable or both.

Thanks,

Mike

_________________________________________

To unsubscribe from OpenMRS Developers' mailing list, send an e-mail to [hidden email] with "SIGNOFF openmrs-devel-l" in the  body (not the subject) of your e-mail.

[mailto:[hidden email]?body=SIGNOFF%20openmrs-devel-l]


_________________________________________

To unsubscribe from OpenMRS Developers' mailing list, send an e-mail to [hidden email] with "SIGNOFF openmrs-devel-l" in the  body (not the subject) of your e-mail.

[mailto:[hidden email]?body=SIGNOFF%20openmrs-devel-l]

---

[hidden email] from OpenMRS Developers' mailing list