aboutsummaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
* Merge pull request #18077 from claudiob/add-docs-for-test-orderRafael Mendonça França2014-12-182-2/+17
|\ | | | | Add docs for AS::TestCase::test_order
| * Add docs for AS::TestCase::test_orderclaudiob2014-12-182-2/+17
|/ | | | | | Document `test_order` and `test_order=` from `ActiveSupport::TestCase`. [ci skip]
* Merge pull request #18087 from georgemillo/patch-2Eileen M. Uchitelle2014-12-181-2/+2
|\ | | | | Spelling fix [ci skip]
| * Spelling fix [ci skip]George Millo2014-12-181-2/+2
|/
* passed a spell checker, not bad [ci skip]Xavier Noria2014-12-181-1/+1
|
* adds another examle to the constants not missing gotcha [ci skip]Xavier Noria2014-12-181-0/+55
| | | | /cc @matthewd
* Relax the UUID regexGodfrey Chan2014-12-182-12/+9
| | | | | | | | Apparently PG does not validate against RFC 4122. The intent of the original patch is just to protect against PG errors (which potentially breaks txns, etc) because of bad user input, so we shouldn't try any harder than PG itself. Closes #17931
* Merge pull request #18081 from yui-knk/fix/capture_helperAbdelkader Boudih2014-12-181-1/+2
|\ | | | | [ci skip] Fix HTML format on comment
| * [ci skip] Fix HTML format on commentyui-knk2014-12-181-1/+2
|/
* Fix typo in nodoc should be `:nodoc:` for RDoc to parse correctlyZachary Scott2014-12-171-1/+1
|
* Merge pull request #18076 from andreynering/guides-changelogZachary Scott2014-12-171-1/+1
|\ | | | | Fix name on changelog [ci skip]
| * Fix name on changelog [ci skip]Andrey Nering2014-12-171-1/+1
|/
* Merge pull request #18063 from claudiob/remove-as-time-with-zone-sinceRafael Mendonça França2014-12-171-10/+1
|\ | | | | Replace AS::TimeWithZone#since with alias to +
| * Replace AS::TimeWithZone#since with alias to +claudiob2014-12-161-10/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Stems from [Google group discussion](https://groups.google.com/forum/#!topic/rubyonrails-core/jSPbP-TNLb0). Currently `AS::TimeWithZone` has two methods to add an interval to a time: `+(other)` and `since(other)` ([docs](http://edgeapi.rubyonrails.org/classes/ActiveSupport/TimeWithZone.html)). The two methods are "pretty much" equivalent in every case: 1. When adding any interval to an `AS::TimeWithZone` representing a `Time`: ```ruby t = Time.now.in_time_zone #=> Thu, 04 Dec 2014 18:56:28 EST -05:00 t + 1 == t.since(1) #=> true t + 1.day == t.since(1.day) #=> true t + 1.month == t.since(1.month) #=> true ``` 2. When adding any interval to an `AS::TimeWithZone` representing a `Date`: ```ruby d = Date.today.in_time_zone #=> Thu, 04 Dec 2014 00:00:00 EST -05:00 d + 1 == d.since(1) #=> true d + 1.day == d.since(1.day) #=> true d + 1.month == d.since(1.month) #=> true ``` 3. When adding any interval to an `AS::TimeWithZone` representing a `DateTime`: ```ruby dt = DateTime.now.in_time_zone #=> Thu, 04 Dec 2014 18:57:28 EST -05:00 dt + 1 == dt.since(1) #=> true dt + 1.day == dt.since(1.day) #=> true dt + 1.month == dt.since(1.month) #=> false ``` As you can see, the only case in which they differ is when the interval added to a `DateTime` is in a format like `1.month`. However, this usage of "since" is explicitly discouraged by the [documentation of `DateTime#since`](https://github.com/rails/rails/blob/master/activesupport/lib/active_support/core_ext/date_time/calculations.rb#L86L88): > Returns a new DateTime representing the time a number of seconds since the instance time. > Do not use this method in combination with x.months, use months_since instead! And indeed, following this recommendation the correct result is returned: ```ruby dt + 1.month == dt.months_since 1 #=> true ``` Therefore, my proposal is to remove the method definition of `TimeWithZone#since` and instead replace it with a simple `alias_method :since, :+`. The rationale is that the only case where they differ is a case that is explicitly discouraged as "wrong". In my opinion, having two methods named `since` and `+` and having to figure out exactly what the difference is makes the codebase more confusing. However, I understand this PR is "subjective", so if you feel like it's better to ignore this, feel free to close the PR. Thanks!
* | Merge pull request #18072 from exAspArk/thread_safe_gem_versionRafael Mendonça França2014-12-171-1/+1
|\ \ | | | | | | | | | Use thread_safe gem version greater or equal to 0.3.4
| * | use thread_safe gem version greater or equal to 0.3.4Evgeny Li2014-12-171-1/+1
| | |
* | | simpler example for the non-missing constants gotcha [ci skip]Xavier Noria2014-12-171-42/+39
|/ / | | | | | | | | | | | | | | | | The previous example was a little convoluted and the exposition claifying the parts that were correct albeit not totally obvious were interferring in my view. This example has less things going on and gets to the key problem with less balls in the air.
* | Merge pull request #18068 from kamipo/remove_unused_lineCarlos Antonio da Silva2014-12-171-1/+0
|\ \ | | | | | | Remove unused line
| * | Remove unused lineRyuta Kamizono2014-12-171-1/+0
| | |
* | | Merge pull request #18069 from yui-knk/fix/guideRafael Mendonça França2014-12-171-3/+3
|\ \ \ | |/ / |/| | [ci skip] Change three backticks to just one
| * | [ci skip] Change three backticks to just oneyui-knk2014-12-171-3/+3
|/ /
* | Merge pull request #18065 from y-yagi/fix_autoloading_guideXavier Noria2014-12-171-1/+1
|\ \ | |/ |/| fix link in autoloading guide [ci skip]
| * fix link in autoloading guide [ci skip]yuuji.yaginuma2014-12-171-1/+1
|/
* `update_column` take ruby-land input, not database-land inputSean Griffin2014-12-164-3/+27
| | | | | | | | | | | | | | | In the case of serialized columns, we would expect the unserialized value as input, not the serialized value. The original issue which made this distinction, #14163, introduced a bug. If you passed serialized input to the method, it would double serialize when it was sent to the database. You would see the wrong input upon reloading, or get an error if you had a specific type on the serialized column. To put it another way, `update_column` is a special case of `update_all`, which would take `['a']` and not `['a'].to_yaml`, but you would not pass data from `params` to it. Fixes #18037
* Merge pull request #17980 from gsamokovarov/rescuable-case-operatorRafael Mendonça França2014-12-163-4/+45
|\ | | | | Add class level case operator support for error dispatching in Rescuable
| * Add class level case operator support for error dispatching in RescuableGenadi Samokovarov2014-12-103-4/+45
| |
* | Merge pull request #18059 from andreynering/ar-guides-queryingRafael Mendonça França2014-12-161-5/+28
|\ \ | | | | | | Improving Method Chaining section [ci skip]
| * | Improving Method Chaining section [ci skip]Andrey Nering2014-12-161-5/+28
|/ /
* | Merge pull request #17995 from ↵Rafael Mendonça França2014-12-162-0/+9
|\ \ | | | | | | | | | | | | jethroo/fix/assert_template_with_unsupported_layout_type assert template should raise ArgumentError for unsupported layout types
| * | adding that assert_template with :layout will raise ArgumentError for ↵Carsten Wirth2014-12-162-0/+9
| | | | | | | | | | | | unknown layout type
* | | Merge pull request #18055 from jonatack/patch-8Zachary Scott2014-12-161-2/+2
|\ \ \ | | | | | | | | "backoffice" -> "back office", "lookup" -> "look up" [ci skip]
| * | | "backoffice" -> "back office", "lookup" -> "look up"Jon Atack2014-12-161-2/+2
|/ / / | | | | | | | | | | | | "Lookup" is a noun, but not a verb. The verb is "look up". [skip ci]
* | | Merge pull request #18049 from yuki3738/fix_rails_db_command_errorRafael Mendonça França2014-12-161-1/+1
|\ \ \ | | | | | | | | Add a code checking about file or not to the rails db command
| * | | Add a code checking about file or not to the rails db commandyuki37382014-12-161-1/+1
| | | |
* | | | Merge pull request #18052 from timoschilling/reset_variantsRafael Mendonça França2014-12-162-1/+9
|\ \ \ \ | | | | | | | | | | allow reseting of request variants
| * | | | allow reseting of request variantsTimo Schilling2014-12-162-1/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The current implementation of `variants=` don't allow a resetting to nil, wich is the default value. This results in the following code smell: ```ruby case request.user_agent when /iPhone/ request.variants = :phone when /iPad/ request.variants = :ipad end ``` With the ability to reset variants to nil, it could be: ```ruby request.variants = case request.user_agent when /iPhone/ :phone when /iPad/ :ipad end ```
* | | | | Merge pull request #18032 from claudiob/add-test-for-after-validate-callbacksRafael Mendonça França2014-12-162-6/+28
|\ \ \ \ \ | | | | | | | | | | | | Add test for ActiveModel `after_validation`, `after_` and `around_` callbacks returning false
| * | | | | Add AM test: after/around callback returning falseclaudiob2014-12-141-4/+15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This stems from https://github.com/rails/rails/pull/17227#discussion_r21641358 It's simply a clarification of the current behavior by which if an `after_` or `around_` ActiveModel callback returns +false+, then the callback chain **is not halted**. The callback chain in ActiveModel is only halted when a `before_` callback returns `false`.
| * | | | | Add AM test for after_validation returning falseclaudiob2014-12-141-2/+13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This stems from https://github.com/rails/rails/pull/17227#discussion_r21641358 It's simply a clarification of the current behavior by which if an `after_validation` ActiveModel callback returns +false+, then further `after_` callbacks **are not halted**.
* | | | | | Merge pull request #18054 from jonatack/add_content_tag_testRafael Mendonça França2014-12-161-0/+5
|\ \ \ \ \ \ | | | | | | | | | | | | | | Test to ensure content_tag works when fixing #17661
| * | | | | | Test to ensure content_tag works when fixing #17661Jon Atack2014-12-161-0/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | After c2fe093, which was reverted yesterday, there will likely be future PRs to address #17661. The test `actionview/test/template/capture_helper_test.rb #test_capture_returns_nil_if_the_returned_value_is_not_a_string` should have errored with c2fe093, but it was rewritten in the PR to not raise. My guess is that it may have seemed irrelevant to the content_tag tests or already covered by them. This test provides additional protection by being in the content_tag test suite to explicitly raise a red flag in future cases. It foregoes some redundancy for safety — at least until #17661 is closed.
* | | | | | | Merge pull request #18031 from claudiob/better-tests-for-callbacks-terminatorRafael Mendonça França2014-12-162-19/+38
|\ \ \ \ \ \ \ | |/ / / / / / |/| | | | | | Add test for `:skip_after_callbacks_if_terminated`
| * | | | | | Add test for `:skip_after_callbacks_if_terminated`claudiob2014-12-142-19/+38
| |/ / / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | `define_callbacks` from `ActiveSupport::Callbacks` accepts the `:skip_after_callbacks_if_terminated` option since #4866 but the option is not tested anywhere. This commit adds tests and fixes documentation for the option, making it clear that halting a callback chain only stops following `before_` and `around_` callbacks by default.
* | | | | | Merge pull request #18050 from jonatack/patch-13Xavier Noria2014-12-161-7/+7
|\ \ \ \ \ \ | | | | | | | | | | | | | | Improve section in constant autoload guide
| * | | | | | Improve section in constant autoload guideJon Atack2014-12-161-7/+7
| | |/ / / / | |/| | | | | | | | | | [skip ci]
* | | | | | Merge pull request #18036 from ↵Rafael Mendonça França2014-12-161-19/+1
|\ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | claudiob/remove-redundant-tests-about-around-actions-returning-false Remove misleading test: around_action return false
| * | | | | | Remove misleading test: around_action return falseclaudiob2014-12-151-19/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When an `around_action` does not `yield`, then the corresponding action is *never* executed and the `after_` actions are *never* invoked. The value returned by the `around_action` does not have any impact on this: an `around_action` can "return" `true`, `false`, or `"pizza"`, but as long as `yield` is not invoked, the corresponding action and after callbacks are not executed. The test suite for `ActionController::Callbacks` currently includes separate tests to distinguish the cases in which a non-yielding `around_actions` returns `true` or `false`. In my opinion, having such tests is misleading, giving the impression that the returned value might have some sort of impact, while it does not. At least that's the impression I got when I read those tests. For completeness, the tests were introduced 7 years ago by @NZKoz in e80fabb.
* | | | | | | Merge pull request #18053 from georgemillo/patch-2Arthur Nogueira Neves2014-12-161-3/+3
|\ \ \ \ \ \ \ | | | | | | | | | | | | | | | | English fix [ci skip]
| * | | | | | | English fix [ci skip]George Millo2014-12-161-3/+3
|/ / / / / / /
* | | | | | | Merge pull request #18051 from jonatack/patch-14Eileen M. Uchitelle2014-12-161-2/+2
|\ \ \ \ \ \ \ | | | | | | | | | | | | | | | | Improve grammar