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

Fixes #7038: No error when hostgroup name exceeds 245 characters #1701

Closed
wants to merge 1 commit into from

Conversation

orrabin
Copy link
Member

@orrabin orrabin commented Aug 20, 2014

This PR replaces #1686

@tstrachota
Copy link
Member

Tested and I'm getting message "is too long (maximum is 255 characters) and Maximum for this name is 245 characters" in the UI. It seems that there's some duplicity.

max_length_for_name = self.send(:obj_type).length + 1
max_length_for_name += parent.title.length unless parent.nil?

errors.add(:name, _("Maximum for this name is #{255 - max_length_for_name} characters")) if 255 - (name.length + max_length_for_name) < 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note you can't put interpolation inside an extracted string (use %s etc), and errors for attributes should begin with a lower case letter so they read correctly when concatenated.

@tstrachota
Copy link
Member

@orrabin can you please also make sure there are tests for the hostgroup name length.

max_length_for_name = self.send(:obj_type).length + 1
max_length_for_name += parent.title.length unless parent.nil?

errors.add(:name, _("Maximum for this name is %s characters") % (255 - max_length_for_name)) if 255 - (name.length + max_length_for_name) < 0
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@domcleal changed to %s but the error is not concatenated with Name so I left the capital letter at the beginning

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be when rendered with full_messages (in the API I think), as you get..

 > Hostgroup.new(:name => 'a' * 300).tap { |h| h.valid? }.errors.full_messages.to_sentence
 => "Title is too long (maximum is 255 characters) and Name Maximum for this name is 245 characters" 

@orrabin
Copy link
Member Author

orrabin commented Aug 28, 2014

@tstrachota I fixed the double message and added some tests

max_length_for_name = self.send(:obj_type).length + 1
max_length_for_name += parent.title.length unless parent.nil?

errors.add(:name, _("maximum for this name is %s characters") % (255 - max_length_for_name)) if 255 - (name.length + max_length_for_name) < 0
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@domcleal I fixed the error message

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tests are now failing

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, should pass now

@orrabin
Copy link
Member Author

orrabin commented Sep 11, 2014

[test]

@ohadlevy
Copy link
Member

merged as 5fbe760 thanks @orrabin

@ohadlevy ohadlevy closed this Sep 15, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants