Skip to content

Bring up to date rails, minitest #17

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

Merged
merged 45 commits into from
Nov 11, 2018
Merged

Bring up to date rails, minitest #17

merged 45 commits into from
Nov 11, 2018

Conversation

wbreeze
Copy link
Contributor

@wbreeze wbreeze commented Dec 4, 2017

This is a mixed bag because I merged some of the improvements from others, for which I think we already have pull requests.

In addition, I ditched rcov and jeweler (but didn't use the bundler method to infer Gemfile from gemspec)

And this computes a unique slug by a more portable method. PostgreSQL wants CAST ... AS INTEGER. MySQL wants CAST ... AS UNSIGNED. And there were a lot of string manipulation tricks involved. Ditched that for query - retry until not found.

  • Advantage: simple and portable
  • Disadvantage: n+1 queries for n existing slugs; however, we expect n to be very small. If n is not small, the application is using the wrong source for slugs, or maybe wants uuid's.

And most importantly for me, brought this up to modern MiniTest and Rails.

This was very quick and dirty and I'm putting this out here pro-forma without much expectation. However, if you think of merging it, I'll try to clear any objections.

jurgens and others added 30 commits July 23, 2013 17:56
Rails 4 was failing with this message:

"The provided regular expression is using multiline anchors (^ or $),
which may present a security risk. Did you mean to use \A and \z, or
forgot to add the :multiline => true option?
AFAIK, CAST AS DECIMAL will work across SQLite, MySQL, Postgres

#11
^ and $ match lines. \A and \z match the start and end of the string.
This prevents an error in Rails 5 (and possibly earlier).
This has different syntax compared to SQLite.
threedaymonk and others added 15 commits February 26, 2018 16:31
This requires two changes:

1. Cast to the integer type (rather than unsigned)
2. Convert anything that doesn't come out of the first replace as a
   series of digits to '0'. This catches the case of the first slug with
   no numeric suffix.
Instead of failing, we'll default to the name of the class, so that
there is always a usable slug.
Before:

    "foo bar" => "foo-bar"
    "foo" => "foo-1"
    "foo" => "foo-1" /!\ Error /!\

After

    "foo bar" => "foo-bar"
    "foo" => "foo"
    "foo" => "foo-1"
Update gems for security flaws
There are several improvements in this branch from @ambit-contacts which
we incorporate here:
- add a generic default option vs. throwing an exception when source column
generates an empty slug (we made this optional; some might prefer to get
the exception)
- additional test for partial matches

In addition:
- reduce number of gem dependencies
- dry-up some of the test setup
- use explicit vs. implicit validation methods
- a tiny bit of trailing whitespace cleanup
@bkoski
Copy link
Owner

bkoski commented Nov 11, 2018

These are all great contributions! Should've done this a long time ago, but will go ahead and merge back to master.

@wbreeze
Copy link
Contributor Author

wbreeze commented Nov 12, 2018

Ben:

This is so cool that you brought-in this work. Thank you very much. Thank you for this great gem.

Doug

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants