aboutsummaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
* Refactor decaffeinate output to more natural/idiomatic javascriptRichard Macklin2018-11-026-279/+228
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | - Remove unnecessary Array.from usages from subscriptions.js These were all Arrays before, so Array.from is a no-op - Remove unnecessary IIFEs from subscriptions.js - Manually decaffeinate sample ActionCable code in comments Here the coffeescript -> ES2015 conversion was done by hand rather than using decaffeinate, because these code samples were simple enough. - Refactor ActionCable.Subscription to avoid initClass - Refactor ActionCable.Subscription to use ES2015 default parameters - Refactor ActionCable.ConnectionMonitor to avoid initClass - Refactor ActionCable.ConnectionMonitor to use shorter variations of null checks - Remove unnecessary code created because of implicit returns in ConnectionMonitor This removes the `return` statements that were returning the value of console.log and those from private methods whose return value was not being used. - Refactor ActionCable.Connection to avoid initClass - Refactor Connection#isProtocolSupported and #isState This addresses these three decaffeinate cleanup suggestions: - DS101: Remove unnecessary use of Array.from - DS104: Avoid inline assignments - DS204: Change includes calls to have a more natural evaluation order It also removes the use of Array.prototype.includes, which means we don't have to worry about providing a polyfill or requiring that end users provide one. - Refactor ActionCable.Connection to use ES2015 default parameters - Refactor ActionCable.Connection to use shorter variations of null checks - Remove return statements that return the value of console.log() in ActionCable.Connection - Simplify complex destructure assignment in connection.js decaffeinate had inserted ``` adjustedLength = Math.max(protocols.length, 1) ``` to be safe, but we know that there has to always be at least one protocol, so we don't have to worry about protocols.length being 0 here. - Refactor Connection#getState The decaffeinate translation of this method was not very clear, so we've rewritten it to be more natural. - Simplify destructure assignment in connection.js - Remove unnecessary use of Array.from from action_cable.js.erb - Refactor ActionCable#createConsumer and #getConfig This addresses these two decaffeinate cleanup suggestions: - DS104: Avoid inline assignments - DS207: Consider shorter variations of null checks - Remove unnecessary code created because of implicit returns in action_cable.js.erb This removes the `return` statements that were returning the value of console.log and those from methods that just set and unset the `debugging` flag. - Remove decaffeinate suggestion about avoiding top-level this In this case, the top-level `this` is intentional, so it's okay to ignore this suggestion. - Remove decaffeinate suggestions about removing unnecessary returns I did remove some of the return statements in previous commits, where it seemed appropriate. However, the rest of these should probably remain because the return values have been exposed through the public API. If we want to break that contract, we can do so, but I think it should be done deliberately as part of a breaking-API change (separate from this coffeescript -> ES2015 conversion) - Remove unused `unsupportedProtocol` variable from connection.js Leaving this would cause eslint to fail - Refactor Subscriptions methods to avoid `for` ... `of` syntax Babel transpiles `for` ... `of` syntax to use `Symbol.iterator`, which would require a polyfill in applications that support older browsers. The `for` ... `of` syntax was produced by running `decaffeinate`, but in these instances a simpler `map` should be sufficient and avoid any `Symbol` issues.
* Run decaffeinate on action_cable/*.jsRichard Macklin2018-11-026-401/+581
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Using [decaffeinate], we have converted these files from coffeescript syntax to ES2015 syntax. Decaffeinate is very conservative in the conversion process to ensure exact coffeescript semantics are preserved. Most of the time, it's safe to clean up the code, and decaffeinate has left suggestions regarding potential cleanups we can take. I'll tackle those cleanups separately. After running decaffeinate, I ran: ``` eslint --fix app/javascript ``` using the eslint configuration from ActiveStorage to automatically correct lint violations in the decaffeinated output. This removed 189 extra semicolons and changed one instance of single quotes to double quotes. Note: decaffeinate and eslint can't parse ERB syntax. So I worked around that by temporarily quoting the ERB: ```diff @ActionCable = - INTERNAL: <%= ActionCable::INTERNAL.to_json %> + INTERNAL: "<%= ActionCable::INTERNAL.to_json %>" WebSocket: window.WebSocket logger: window.console ``` and then removing those quotes after running decaffeinate and eslint. [decaffeinate]: https://github.com/decaffeinate/decaffeinate
* Move actioncable javascript to app/javascript and change .coffee -> .jsRichard Macklin2018-11-026-0/+0
| | | | | | | | | | | | | | | | | | | | | - Rename action_cable/*.coffee -> *.js - Move app/assets/javascripts/* -> app/javascript/* - Rename action_cable.js.erb -> action_cable/index.js.erb Renaming the extension to .js is in preparation for converting these files from coffeescript to ES2015. Moving the files to app/javascript and putting the entry point in index.js.erb changes the structure of ActionCable's javascript to match the structure of ActiveStorage's javascript. (We are doing the file moving and renaming in a separate commit to ensure that the git history of the files will be preserved - i.e. git will track these as file renames rather than unrelated file additions/deletions. In particular, git blame will still trace back to the original authorship.)
* Merge pull request #34360 from ↵Rafael França2018-11-013-0/+13
|\ | | | | | | | | weilandia/34359_hash_with_indifferent_access_to_options Make ActiveSupport HashWithIndifferentAccess#to_options and alias for HashWithIndifferentAccess#symbolize_keys
| * Make #to_options an alias for #symbolize_keysNick Weiland2018-11-013-0/+13
| | | | | | | | | | | | | | | | | | | | | | | | Fixes #34359 Prior to 5.2.0 (2cad8d7), HashWithIndifferentAccess#to_options acted as an alias to HashWithIndifferentAccess#symbolize_keys. Now, #to_options returns an instance of HashWithIndifferentAccess while #symbolize_keys returns and instance of Hash. This pr makes it so HashWithIndifferentAccess#to_options acts as an alias for HashWithIndifferentAccess#symbolize_keys once again.
* | Merge pull request #34361 from esparta/update_sample_callback_before_destroyRyuta Kamizono2018-11-011-1/+1
|\ \ | |/ |/| Updating sample code on ActiveRecord#before_destroy callback [ci skip]
| * Updating sample code on ActiveRecord#before_destroy callback [ci skip]Espartaco Palma2018-10-311-1/+1
|/ | | | It was executing a delete_all method with wrong parameter
* Merge pull request #34356 from gmcgibbon/docs_i18n_with_locale_threadsafeRafael França2018-10-311-15/+23
|\ | | | | Make i18n locale setting docs use around_action
| * Make i18n locale setting docs use around_actionGannon McGibbon2018-10-311-15/+23
| | | | | | | | | | | | | | | | | | Changes `I18n.locale` assignment in docs to use `I18n.with_locale` in `around_action` to ensure locale resetting after action processing. [ci skip] [Gannon McGibbon + Leonardo Tegon]
* | Fix "warning: shadowing outer local variable - role"Ryuta Kamizono2018-10-311-9/+13
| | | | | | | | Caused at #34196.
* | Merge pull request #34355 from esquith/patch-1Gannon McGibbon2018-10-301-1/+1
|\ \ | | | | | | Update authorization.rb
| * | Update authorization.rbEsquith Allen2018-10-301-1/+1
| | |
* | | Restore HWIA support to AJ::Arguments.deserializeGannon McGibbon2018-10-303-1/+13
| | | | | | | | | | | | | | | Restore HashWithIndifferentAccess support to ActiveJob::Arguments.deserialize.
* | | Merge pull request #34353 from gmcgibbon/fix_lock_docs_locking_clauseRafael França2018-10-301-3/+3
|\ \ \ | |/ / |/| | Fix example for database-specific locking clause
| * | Fix example for database-specific locking clauseGannon McGibbon2018-10-301-3/+3
| |/ | | | | | | [ci skip]
* | Merge pull request #33972 from bogdanvlviv/follow-up-33897Rafael França2018-10-302-9/+13
|\ \ | | | | | | Improve `enqueue_retry.active_job` message
| * | Improve `enqueue_retry.active_job` messagebogdanvlviv2018-09-252-9/+13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since #33751 was added `enqueue_retry.active_job` instrumentation to the `retry_on` method, then #33897 moved the instrumentation to `retry_job` method in order to ensure that this method publish `enqueue_retry.active_job` notification too. See related discussion https://github.com/rails/rails/pull/33751#discussion_r214140008 Since `enqueue_retry.active_job` moved to `retry_job`, there is no guarantee that payload of `enqueue_retry.active_job` would have `:error`. See test `LoggingTest#test_enqueue_retry_logging_on_retry_job` as example of that case. Related to https://github.com/rails/rails/pull/33897#discussion_r219707024 I think we can improve notification of `enqueue_retry.active_job`: - If there is no `event.payload[:error]`, then publish like "Retrying RescueJob in 3 seconds." only. - If `event.payload[:wait]` is `nil`, then publish "Retrying RescueJob in 0 seconds." instead of "Retrying RescueJob in nil seconds." - If there is `event.payload[:error]`, then publish "Retrying RescueJob in 3 seconds, due to a DefaultsError.". - Change the type of the message from `error` to `info.` Also, this commit removes part of messages - "The original exception was #{ex.cause.inspect}." of `enqueue_retry.active_job`, `retry_stopped.active_job`, and `discard.active_job` since I haven't found it useful. Please let me know whether you agree with that?
* | | Fix failing testSean Griffin2018-10-301-0/+1
| |/ |/| | | | | | | b63701e moved the assignment before the query, but we need to capture our old id before assignment in case we are assigning the id.
* | `update_columns` raises if the column is unknownSean Griffin2018-10-304-4/+16
| | | | | | | | | | | | | | | | | | Previosly, `update_columns` would just take whatever keys you gave it and tried to run the update query. Most likely this would result in an error from the database. However, if the column actually did exist, but was in `ignored_columns`, this would result in the method returning successfully when it should have raised, and an attribute that should not exist written to `@attributes`.
* | Merge pull request #34314 from bf4/patch-2Gannon McGibbon2018-10-301-1/+1
|\ \ | | | | | | ActionController::API *does* support cookies, sessions
| * | ActionController::API *does* support cookies, sessionsBenjamin Fleischer2018-10-251-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | ActionController::Metal provides session support by delegating `session to the request (`"@_request"`) https://github.com/rails/rails/blob/a3dcba42e2422eb9c2e77011a39ce72dc934b420/actionpack/lib/action_controller/metal.rb#L149 Though the ActionController::Cookies modules isn't included, it's really a convenience for providing a first class `cookies` object. *all* ActionController::Metal subclasses support setting cookies via the `session` object.
* | | Merge pull request #34349 from yahonda/restore_encoding_utf8mb4Ryuta Kamizono2018-10-302-0/+2
|\ \ \ | | | | | | | | Restore `encoding: utf8mb4` in database.yml
| * | | Restore `encoding: utf8mb4` in database.ymlYasuo Honda2018-10-302-0/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | rails/rails#33853 and rails/rails#33929 removed `encoding: utf8mb4` from database.yml since at that time MySQL 5.1 is supported with the master branch. Since MySQL 5.1 has been dropped, we can restore `encoding: utf8mb4` in database.yml
* | | | Merge pull request #19388 from yakara-ltd/fix-habtm-fixture-orderRyuta Kamizono2018-10-302-0/+7
|\ \ \ \ | | | | | | | | | | Avoid violating key constraints in fixture HABTM associations
| * | | | Avoid violating key constraints in fixture HABTM associationsJames Le Cuirot2018-10-292-0/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When loading fixtures, Ruby 1.9's hash key ordering means that HABTM join table rows are always loaded before the parent table rows, violating foreign key constraints that may be in place. This very simple change ensures that the parent table's key appears first in the hash. Violations may still occur if fixtures are loaded in the wrong order but those instances can be avoided unlike this one.
* | | | | Merge pull request #34196 from gmcgibbon/connection_switch_string_nameEileen M. Uchitelle2018-10-303-6/+63
|\ \ \ \ \ | | | | | | | | | | | | Add support for hash and url configs to be used in connected_to
| * | | | | Add support for hash and url configs in connected_toGannon McGibbon2018-10-263-6/+63
| | |/ / / | |/| | | | | | | | | | | | | | | | | | Add support for hash and url configs in database hash of `ActiveRecord::Base.connected_to`.
* | | | | Exercise HABTM fixtures with foreign key constraintsRyuta Kamizono2018-10-302-45/+52
| | | | |
* | | | | We don't want these internal methods as public methods in our controllersAkira Matsuda2018-10-301-23/+25
| | | | | | | | | | | | | | | | | | | | or they would be listed in `action_methods`
* | | | | Missing require "active_support/executor"Akira Matsuda2018-10-301-0/+1
| | | | |
* | | | | Missing require "core_ext/module/attribute_accessors"Akira Matsuda2018-10-301-0/+1
| | | | |
* | | | | Missing require "active_support/concern"Akira Matsuda2018-10-301-0/+2
| | | | |
* | | | | Merge pull request #34338 from ↵Ryuta Kamizono2018-10-302-4/+4
|\ \ \ \ \ | |_|/ / / |/| | | | | | | | | | | | | | malclocke/document_exception_class_for_restrict_with_exception Document exception from restrict_with_exception [ci skip]
| * | | | Document exception from restrict_with_exception [ci skip]Malcolm Locke2018-10-292-4/+4
| | | | |
* | | | | Merge pull request #34335 from robin850/update-bug-report-templatesRyuta Kamizono2018-10-2911-87/+11
|\ \ \ \ \ | | | | | | | | | | | | Update bug report templates
| * | | | | Update bug report templatesRobin Dupret2018-10-2811-87/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Given that Bundler 1.10 was released back in 2015, we can assume that people are at least on this version or a higher one so there is no need to ask people to upgrade. Also, given that Rails 5.0 supports Ruby 2.2+ and given that this version come with Minitest 5.4.3 bundled, we don't need to ensure backward compatibility with Minitest 4.
* | | | | | Merge pull request #34334 from ↵Ryuta Kamizono2018-10-291-2/+3
|\ \ \ \ \ \ | |_|/ / / / |/| | | | | | | | | | | | | | | | | albertoalmagro/privatize-constants-activesupport-timezone Privatize and add # :nodoc: to constants
| * | | | | Privatize and add # :nodoc: to constantsAlberto Almagro2018-10-281-2/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Both `UTC_OFFSET_WITH_COLON` and `UTC_OFFSET_WITHOUT_COLON` are only used within `ActiveSupport::TimeZone` and in my opinion they do not provide relevant information that should appear in the docs.
* | | | | | Registers e302725 in the CHANGELOG [ci skip]Xavier Noria2018-10-281-0/+4
| | | | | | | | | | | | | | | | | | | | | | | | References #34253.
* | | | | | Merge pull request #34333 from albertoalmagro/fix-activemodel-dirty-changed-docRobin Dupret2018-10-281-1/+1
|\ \ \ \ \ \ | | | | | | | | | | | | | | Fix grammar in changed? docs [ci skip]
| * | | | | | Fix grammar in changed? docs [ci skip]Alberto Almagro2018-10-281-1/+1
| |/ / / / / | | | | | | | | | | | | | | | | | | See https://english.stackexchange.com/questions/23218/anyone-has-or-anyone-have-seen-them
* | | | | | Merge pull request #34253 from ↵Xavier Noria2018-10-285-1/+52
|\ \ \ \ \ \ | |/ / / / / |/| | | | | | | | | | | | | | | | | haberbyte/fix_autoload_on_activestorage_downloader Fix autoloading issue with ActiveStorage::Downloader
| * | | | | Improve the logic that detects non-autoloaded constantsJan Habermann2018-10-285-1/+52
|/ / / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If you require `nokogiri` from `app/models/user.rb`, dependencies.rb does not mark `Nokogiri` as an autoloaded constant, as expected. But the logic to detect these non-autoloaded constants is incomplete. See the tests defined in the patch for some cases incorrectly handled.
* | | | | Use release dates for versions in Rails Guides indexOlivier Lacan2018-10-271-11/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We repeat the phrase "Release Notes" even though release note are listed under a... "Release Notes" category already. With this change, instead of repeating ourselves, we can give people a sense of the recency of each release by simply listing the month and year of each release. This is a follow-up to #34265 as suggested by @jeremy. [ci skip]
* | | | | `exists?` with string argument is not invalid typeRyuta Kamizono2018-10-271-5/+13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Any type can be a primary key, so blank string is also valid value. Closes #26356.
* | | | | `create_table` with `:primary_key` option has no effect if `id: false` is givenRyuta Kamizono2018-10-273-8/+6
| | | | | | | | | | | | | | | | | | | | Use column definition with `primary_key: true` instead.
* | | | | Merge pull request #34329 from r7kamura/feature/where-exclamation-consistencyRyuta Kamizono2018-10-272-1/+5
|\ \ \ \ \ | |_|/ / / |/| | | | Ignore empty condition on #construct_relation_for_exists
| * | | | Ignore empty condition on #construct_relation_for_existsr7kamura2018-10-272-1/+5
|/ / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | At https://github.com/rails/rails/commit/fc0e3354af7e7878bdd905a95ce4c1491113af9a, ```rb relation = relation.where(conditions) ``` was rewritten to: ```rb relation.where!(condition) ``` This change accidentally changed the result of `Topic.exists?({})` from true to false. To fix this regression, first I moved the blank check logic (`opts.blank?`) from `#where` to `#where!`, because I thought `#where!` should be identical to `#where`, except that instead of returning a new relation, it adds the condition to the existing relation. But on second thought after some discussion on https://github.com/rails/rails/pull/34329, I started to think that just fixing `#construct_relation_for_exists` is more preferable than changing `#where` and `#where!`.
* | | | Merge pull request #34266 from esquith/improve-no-response-file-wordingEileen M. Uchitelle2018-10-261-5/+4
|\ \ \ \ | | | | | | | | | | Improve no-response file wording
| * | | | Changed wording for no-response documentationEsquith Allen2018-10-201-5/+4
| | | | |