Return Authorization Module (RMA)

[dw08gm]

Some of your suggestions removes some of the flexibility Clyde was trying to build in..

1. Button versus a link on the account histroy page, the inclusion of this code should be a flexible admin configurable choice IMHO. I'll take a look at how to make that so.. Anfd the text should DEFINITELY not be hard coded in.. It should be in a proper language definition file.
2. Regarding item #2 (mods to includes\templates\YourTemplate\templates\tpl_returns_default.php, and \includes\languages\english\YourTeplate\returns.php)
   
   • The extra code Clyde included is for form error validation. Not sure from a usability POV that this should change. Clyde is not here to confirm this, but from a usability POV, I can only assume he felt that it made more sense to put the field level error messages next to each field versus including them all in the message stack. He opted instead for a simpler single message stack error message with field level error messages.
   • It would be a less than satisfying user experience to present the shopper with only one error message "Errors have occured on your submission! Please correct and re-submit" without indicating WHICH field is in error, and HOPING they will "figure it out" based on a simple "Required" symbol. Clyde's approach eliminates the possiblity of user frustration and lends to a better user experience.
   • I'm not sure that the the code from the Capitalize_Signup_Fields mod should be included.. Seems more of a personal choice and as much as possible I'm a BIG proponent that modules should be flexible. Not sure it's practical to make this an admin configurable option either, but I will take a look.. Honestly if folks want this feature they really should just install the Capitalize_Signup_Fields mod themselves
   • Replacing the send button with the submit button.. I'll have to look at this and see how the contact form is configure.. at the very LEAST the RMA form and the contact form should use the same submit/send button.
   • Will take a look at the returns message.. Again, Clyde is not here to confirm what he did, but given the code, it appears he was leaning towards a more flexible solution by giving the admin more options to control the look and feel.. Your changes are a personal choice, and while I don't think they should be part of the codebase, perhaps I could take a look to see if the current options could allow a shopowner to make similar changes without removing ALL of the current code as you have done or provide the means for shopowners to make similar changes if the current codebase won't accomodate this. there are folks who have been using this module who LIKE the returns message the way it is.. This module should support choices and flexibility with regards to look and feel.
   
   
3. Adding the comment is a good idea.. I will also include this in the readme as not all shopowners (especially the DIYers) are going to KNOW to look for this comment. It might even be a BETTER idea to try and make this an admin configurable option so that DIYer shopowners don't HAVE to mess with the code to make this change..

Clyde added a whole lot of things more to satisfy requests from individuals rather than as great leaps forwards. The error messaging and required options thingies should also be considered personal choice things, as neither really improved existing functionality. As I am not into change for change sake, and prefer simplicity over bells and whistles, I begrudgingly had to spend a few days restoring the latest offerings of this mod to what I had and liked with RMA 2.3.2/2.3.3.

Yes, Capitalize_Signup_Fields is entirely optional. It was only included in my post because it was already in my code. I tend to fill out and send forms all in lowercase, and find auto-capitalisation helpful in scanning for input errors.

One thing I would like to know, however, is whether the following highlighted code is entirely necessary, as I cannot recall exactly where it came from, except that it was around 139h days.

<label class="inputLabel" for="contactname"><?php echo ENTRY_NAME . '<span class="reqd">' . ENTRY_REQUIRED_SYMBOL . '</span>'; ?></label>
<?php echo zen_draw_input_field('contactname', zen_output_string_protected($name), ' size="46" id="contactname" onchange="capitalize(this,1);"'); ?>

Cheers

[rbarbour]

Clyde added a whole lot of things more to satisfy requests from individuals rather than as great leaps forwards. The error messaging and required options thingies should also be considered personal choice things, as neither really improved existing functionality. As I am not into change for change sake, and prefer simplicity over bells and whistles, I begrudgingly had to spend a few days restoring the latest offerings of this mod to what I had and liked with RMA 2.3.2/2.3.3.

After looking at a dozen or so RMA Request forms, IMHO, I would say Clyde nailed it with his choices as to which fields needed to be required and which should be optional.

One thing I would like to know, however, is whether the following highlighted code is entirely necessary, as I cannot recall exactly where it came from, except that it was around 139h days.

<label class="inputLabel" for="contactname"><?php echo ENTRY_NAME . '<span class="reqd">' . ENTRY_REQUIRED_SYMBOL . '</span>'; ?></label>
<?php echo zen_draw_input_field('contactname', zen_output_string_protected($name), ' size="46" id="contactname" onchange="capitalize(this,1);"'); ?>

I don't recall ever seeing that code in any version.

[DivaVocals]

Clyde added a whole lot of things more to satisfy requests from individuals rather than as great leaps forwards. The error messaging and required options thingies should also be considered personal choice things, as neither really improved existing functionality.

Respectfully I do disagree with you..

Clyde had had solid knowledge of what makes for good usability. It is reflected in his choice to make the validation errors appear at the field level instead of the message stack.

In form validation it IMPROVES usability if you INFORM users of WHICH fields they missed in filling out a form.. Your proposed changes will REMOVE that functionality leaving them to GUESS which field they missed.. There are 10 required fields on an RMA form.. Adding multiple error messages to the message stack is not a good option from a usability POV. Highlighting the MISSED fields with a clear on-screen message is following good usability practices. I believe this is what Clyde was attempting to achieve. Your suggested code not only removes the usability aspect, but it leaves the customer guessing which field is the offending field. Do not assume that it's OBVIOUS. It's only somewhat obvious which fields are required.. what's not obvious is which field failed the validation.. and without a specific error message guiding the end user, this will lead to a frustrating user experience..

Clearly you do not agree, and that's fine..

As I am not into change for change sake, and prefer simplicity over bells and whistles, I begrudgingly had to spend a few days restoring the latest offerings of this mod to what I had and liked with RMA 2.3.2/2.3.3.

It's your choice of course..

Yes, Capitalize_Signup_Fields is entirely optional. It was only included in my post because it was already in my code. I tend to fill out and send forms all in lowercase, and find auto-capitalisation helpful in scanning for input errors.

Right.. again it's a seperate mod and if someone really wants the feature, they should install the mod..

One thing I would like to know, however, is whether the following highlighted code is entirely necessary, as I cannot recall exactly where it came from, except that it was around 139h days.

<label class="inputLabel" for="contactname"><?php echo ENTRY_NAME . '<span class="reqd">' . ENTRY_REQUIRED_SYMBOL . '</span>'; ?></label>
<?php echo zen_draw_input_field('contactname', zen_output_string_protected($name), ' size="46" id="contactname" onchange="capitalize(this,1);"'); ?>

Cheers

At work.. can't see this.. don't recall this code specifically..

[DivaVocals]

One thing I would like to know, however, is whether the following highlighted code is entirely necessary, as I cannot recall exactly where it came from, except that it was around 139h days.

<label class="inputLabel" for="contactname"><?php echo ENTRY_NAME . '<span class="reqd">' . ENTRY_REQUIRED_SYMBOL . '</span>'; ?></label>
<?php echo zen_draw_input_field('contactname', zen_output_string_protected($name), ' size="46" id="contactname" onchange="capitalize(this,1);"'); ?>

Cheers

I've looked at both v2.3.3 and v2.3.3a and v2.3.2. This code doesn't exist in any of these versions.. Given that you cherrypicked out the changes you wanted and customized RMA, it's seems likely that this is possibly code you introduced into your files..

[dw08gm]

Re: zen_output_string_protected

The function has to do with averting "sql-injection" and "XSS vulnerability" in Zen Cart.

The function is still widely used in 151 on both admin and catalog sides, including a small number of header_php.php and tpl_whatever_default.php files on catalog side, including tpl_account_history_info_default.php.

However I remain unsure whether it should be applied to the RMA files, or any form-producing files for that matter, but suspect it should where inputs are extracted from or written to the database by the form files, or where other interacting files or mod, such as tpl_account_history or Email_Archive_Manager, are involved. However, its presence does not seem to affect the processing of the forms.


I found this thread dating from 25 October 2008, although it is unclear to me whether the parts highlighted are relevant to RMA.
http://www.zen-cart.com/showthread.p...ring_protected
Post 3 Dr Byte

If the customer has entered text information for an attribute field such as a filename or a text-input field, if that information is to be displayed again for verification/edit, you certainly want that information sanitized before it's displayed.

Would the redisplay of inputs (eg info inserted into RMA form fields from database, eg oID, name), say in going from the account_history_info_default to the RMA page, or proceeding from input to error checking to error correction in the RMA page, constitute a "displayed again for verification/edit" case?

Post 7 Dr Byte

I would think that the output-protected approach should be run anytime the content of user-collected data is being re-displayed, so that if any sql-injection or other attack would be averted.

Would the redisplay of inputs (eg info inserted into RMA form fields from database, eg oID, name), say in going from the account_history_info_default to the RMA page, or proceeding from input to error checking to error correction in the RMA page, constitute an "output-protected approach should be run anytime the content of user-collected data is being re-displayed" case?

Post 9 Dr Byte

Will have to do some further investigation. Here's a related post: http://www.zen-cart.com/forum/showthread.php?t=64115

This last thread dates from 25 April 2007 and specifically refers to XSS vulnerability in and provides fixes for Zen Cart 1.3.7 (and prior versions).


Re: error messaging
I found displaying error messaging within the form, as opposed to displaying same within a message stack, disrupts the layout of the form - with longer error messages causing greater disruption - which can make the overall form difficult to rescan for errors or alterations.

As online forms and associated error message stacks have been around for ages, I am sure the majority of internet users are quite familiar with navigating their way through them. So from my long experience, displaying error messaging within the form is just a change for change sake.

Let us not lose sight of the fact that we are only talking about a Returns form that is applied post-purchase. Sure, it may be convenient for customer relations, but displaying error messaging within a form does not provide for greater sales.

cheers

[DivaVocals]

As online forms and associated error message stacks have been around for ages, I am sure the majority of internet users are quite familiar with navigating their way through them. So from my long experience, displaying error messaging within the form is just a change for change sake.

Let us not lose sight of the fact that we are only talking about a Returns form that is applied post-purchase. Sure, it may be convenient for customer relations, but displaying error messaging within a form does not provide for greater sales.

cheers

It does provide for BETTER usability.. It's a frustrating user experience to fill out a form only to find out that it's missing data, and to have NO indication what the issue is... You cannot assume that people will just KNOW and figure it out because you assume that they SHOULD know.

Again we'll have to agree to disagree.. A returns form by it's very nature isn't helping sales, so I fail to see how providing the end user some indication that the form is not filled out correctly correlates to sales.. Displaying a generic validation message without ANY indication where the error in fact lies is not a great user experience.. In my long experience working in software development, when a form is submitted without all the required information you do need to display some indication to the end user where that error lies.. You call it change for change sake, but improving user experience is not in my opinion a bad thing..

Again clearly you do not agree with this logic..

[rbarbour]

Whether it be via message-stack or inline form error messages, these error messages need to be displayed one way or another (if you want required fields).

This mod uses inline error messages, IMHO, i do not think this was change for change sake, with so many forms using CSS tool-tips and jQuery to display inline error messages, I think it was to give the form a more up-to-date presence.

If one chooses to convert to message-stack by all means, try my MessageStack LightAlert Light Box.

I have to agree with @DivaVocals, the inline error messages do provide better usability hence all the new CSS tool-tips and jQuery form builders.

A Return form DOES help sales and confidence with a customer IMHO.

[dw08gm]

A Return form DOES help sales and confidence with a customer IMHO.

We are not disputing the value of Return forms, but rather only where the error messages in the updated mod are to be placed.

My comment regarding form layout dirsuption seems to have been overlooked.

cheers

[rbarbour]

We are not disputing the value of Return forms, but rather only where the error messages in the updated mod are to be placed.

My comment regarding form layout dirsuption seems to have been overlooked.

cheers

Not overlooked, I ran into the same issue on a site I updated - def something to reconsider as far as adding constraints for the displayed (span) error which should be easily accomplished as I used css.

[DivaVocals]

My comment regarding form layout dirsuption seems to have been overlooked.

cheers

Not sure what you are referring to here.. what layout "disruption" (not sure if this is misspelled or not in your post) are you referring to???