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

Change regex not letting utf-8 garden names save #1641

Merged
merged 24 commits into from Nov 11, 2019

Conversation

jenkr55
Copy link
Collaborator

@jenkr55 jenkr55 commented May 20, 2018

Closes #1634

I removed the regex that wasn't letting UTF-8 chars save.

@Br3nda
Copy link
Member

Br3nda commented May 20, 2018

i like this. Anyone know why we stopped it? Was it just because it goes in the url?

@jenkr55
Copy link
Collaborator Author

jenkr55 commented May 20, 2018

Possibly? It still seems to work with special characters unless you can find an example where it doesn't. I also need to fix one more test before this is ready

@pozorvlak
Copy link
Member

What happens if you create a garden containing a character which is not allowed in URLs? Is Rails' slug-making algorithm smart enough to escape them?

@Br3nda
Copy link
Member

Br3nda commented May 21, 2018

If we allow new lines in garden names, what is the consequences? I think just the csv render types -- would we render the new lines on pages? they don't really make sense to me to have a new line in the garden name.

@jenkr55
Copy link
Collaborator Author

jenkr55 commented May 27, 2018

Looks like it's smart enough to not enter newlines and other breaking things in CSV format (just the actual characters for "\n" etc) and URLs seem fine, but to be safe I added a regex (that also allows utf-8)

Br3nda
Br3nda previously approved these changes May 28, 2018
@jenkr55 jenkr55 changed the title Remove regex not letting utf-8 garden names save Changeg regex not letting utf-8 garden names save May 28, 2018
@jenkr55 jenkr55 changed the title Changeg regex not letting utf-8 garden names save Change regex not letting utf-8 garden names save May 28, 2018
@Br3nda
Copy link
Member

Br3nda commented May 28, 2018

merge when ready

Copy link

@milesgould milesgould left a comment

Choose a reason for hiding this comment

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

What strings do we actually want to allow? I suggest the following characters should be allowed:

  • any Unicode letter character
  • combining characters
  • digits
  • most punctuation characters
  • spaces

... but no tabs, newlines, DELs, bells, or NULLs. Anything else we want to exclude?

spec/models/garden_spec.rb Outdated Show resolved Hide resolved
app/models/garden.rb Outdated Show resolved Hide resolved
Copy link
Member

@cesy cesy left a comment

Choose a reason for hiding this comment

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

Please can you address the comments from Miles? Then this should be good to go.

@Br3nda Br3nda added this to In Progress in Growstuff Apr 8, 2019
@Br3nda Br3nda moved this from In Progress to Ready in Growstuff Jul 16, 2019
@always-be-closing
Copy link

7d9cbe9 CodeFactor — 6 issues found. Autofix available. Details

@always-be-closing
Copy link

7d9cbe9 codeclimate — 6 issues to fix Details

@Br3nda Br3nda moved this from Ready to In Progress in Growstuff Nov 2, 2019
@Br3nda Br3nda merged commit 130febb into Growstuff:dev Nov 11, 2019
Growstuff automation moved this from In Progress to Done Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Growstuff
  
Done
Development

Successfully merging this pull request may close these issues.

Allow utf garden names
6 participants