Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixed bug by increasing character count #175

Merged
merged 3 commits into from
Jun 29, 2017

Conversation

jpearce185
Copy link
Contributor

I increased the maximum number of characters allowed in the 'Name' field of the contact forms.

Description

I changed the hard-coded number for maximum character count (100) allowed in the 'Name' field of the contact forms to 130.

Motivation and Context

If a user had a first and last that is the maximum number of characters long (50 each), and then tried to fill out a contact form, it would say underneath the 'Name' field that the maximum number of characters (100) has been exceeded because the form automatically adds the first name, a space, then the last name (totaling 101). This would force the user to have to delete characters in their name. I increased the maximum number of characters allowed in the field as a simple fix and now any user that would have a maximum length first and last name can easily fill out a contact form and submit it.

Tests performed

I opened up a contact form to fill out and in the 'Name' field, it states the maximum number of characters is 130. I also created a test account with a first and last name that contained the maximum number of characters each (50 each: 100 total). I then logged in as that user and opened up a contact form to which the 'Name' and 'Email' fields automatically get filled in, and I was able to submit the form with the 'Name' field consisting of 101 total characters (including the space).

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project as found in the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@jpwhite4
Copy link
Member

What happens if a user has more than 130 characters in their name? What is the impact of this limit change on the rest of the code? Will the (now) longer string cause a problem in some other part of the XDMoD software?

@jpearce185
Copy link
Contributor Author

A user cannot have more than 130 characters in their name because when you sign up and become a user, your name (first and last) total up to 100 characters (50 each), and will not allow either your first or last name to be longer than 50 characters each. Therefore, if a user were to fill out the form they could only ever have a max of 101 characters (because the space counts) and no greater before this limit change.

The point at which more than 101 characters would be used is if:
A) A user adds extra characters to their name, or
B) A guest (someone without an account) fills out a contact form and enters more than 101 characters, in which case if they wanted an account would have to have a 50 character each first and last name

No other code is affected by this change as this limit is only used for the 'Name' field of the contact forms.

@smgallo
Copy link
Contributor

smgallo commented Jun 28, 2017

The name in the contact form is populated by calling https://storage-demo.ccr.xdmod.org/rest/v1/users/current, which returns the first and last name. Both of these have a maximum length of 50 in the database and they are concatenated together when setting the name in the contact form:

txtName.setValue(
    data.results.first_name + ' ' + data.results.last_name
);

Rather than changing the global name length to 130 and risking adverse effects elsewhere, wouldn't it be a better solution to change the text field validation from maxLength: maxNameLength to maxLength: maxNameLength + 1 since the code is adding an extra space?

@jpearce185
Copy link
Contributor Author

I pushed that "lower character limit' commit just before I saw your comment @smgallo; my apologies. However, yes, that is a better solution, and I will make the change.

@jpwhite4 jpwhite4 merged commit 3c4d69f into ubccr:xdmod7.0 Jun 29, 2017
@tyearke tyearke added the bug Bugfixes label Aug 14, 2017
@tyearke tyearke added this to the v7.0.0 milestone Aug 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bugfixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants