Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Rails is not Django #1

Closed
leah opened this Issue Apr 1, 2009 · 11 comments

Comments

Projects
None yet

leah commented Apr 1, 2009

I have an issue with this.

That is a big issue.

dodeja commented Apr 16, 2009

This is a problem.

I’m glad it isn’t.

nofxx commented Apr 17, 2009

I`m sure someone is working on a fix.

rabble commented Apr 17, 2009

Please don’t fix.

mattly commented Apr 17, 2009

-1 please don’t fix. don’t you python people have anything better to do? like scoop up pony power poop?

Contributor

dmathieu commented Apr 21, 2009

Djan What ? How do you even pronounce it ??

@ghost ghost pushed a commit to Mochaleaf/rails that referenced this issue Jul 23, 2011

@leepfrog leepfrog rails#759 -
There are two issues here:

#1) ActiveSupport::Date current method has a misleading conditional. Time.zone is always returned because config.time_zone is set by default on a project.

#2) ActiveSupport::Date does not override the Date.today method. This means that the core ruby library's .today method is called which may be based off of a different time zone than what's set in config.time_zone.

Submitting a pull request.
151723e

@ghost ghost pushed a commit to Mochaleaf/rails that referenced this issue Aug 4, 2011

@leepfrog leepfrog Fixing bugs in ActiveSupport::Date. Fixes #759.
There are two issues here:

#1) ActiveSupport::Date current method has a misleading conditional. Time.zone is always returned because config.time_zone is set by default on a project.

#2) ActiveSupport::Date does not override the Date.today method. This means that the core ruby library's .today method is called which may be based off of a different time zone than what's set in config.time_zone.
2d54abd

archonic commented Jan 2, 2013

lulz

@joergschray joergschray added a commit to tandem-softworks/rails that referenced this issue Apr 27, 2013

@joergschray @joergschray joergschray + joergschray Issue #1: :before_add callback is called when nested attributes assig…
…nment assigns to existing record if the association is not yet loaded

Issue #2: Nested Attributes assignment does not affect the record in the association target when callback triggers loading of the association

Complete rewrite of tests
1eced51

@Empact Empact added a commit to Empact/rails that referenced this issue Aug 12, 2013

@joergschray @Empact joergschray + Empact Fix interactions between :before_add callbacks and nested attributes …
…assignment

Issue #1: :before_add callback is called when nested attributes assignment assigns to existing record if the association is not yet loaded
Issue #2: Nested Attributes assignment does not affect the record in the association target when callback triggers loading of the association
018697d

@kbrock kbrock pushed a commit to kbrock/rails that referenced this issue Dec 23, 2013

@jrafanie jrafanie Merge pull request #1 from lfu/eager_loading_1018392
Fix an issue with where clause with a dot character.
1ca8b5e

@mislav mislav pushed a commit to mislav/rails that referenced this issue Jan 9, 2014

@atmos atmos Merge pull request #1 from github/cve-2013-0333
Backport Patches for CVE-2013-0333
0ad8634

@kassio kassio pushed a commit to kassio/rails that referenced this issue Apr 22, 2014

@brianstorti brianstorti Merge pull request #1 from kassio/bs-kb-fix-primary-key
Add some tests to drive us better
64a8404

@seuros seuros referenced this issue in seuros/django Aug 12, 2014

@dhh dhh Merge pull request #1 from charliesome/patch-1
Clean up JobWrappers::ResqueWrapper.perform
ce124a1

@rafaelfranca rafaelfranca pushed a commit that referenced this issue Oct 15, 2014

@claudiob claudiob Remove duplicate stringify_keys in text_field_tag
All the methods that invoke `text_field_tag` (such as `hidden_field_tag`)
and all the methods that invoke `number_field_tag` (that is `range_field_tag`)
do not need to call `stringify_keys` on their `options` parameter since the
`text_field_tag` method [is already doing it internally](https://github.com/claudiob/rails/blob/4159134524f4c78d008eef9d9a17f73a3172dcc8/actionview/lib/action_view/helpers/form_tag_helper.rb#L182):

```ruby
def text_field_tag(name, value = nil, options = {})
  tag :input, { "type" => "text", "name" => name, "id" => sanitize_to_id(name), "value" => value }.update(options.stringify_keys)
end
```

and `number_field_tag` is [already doing it internally](https://github.com/claudiob/rails/blob/e3207bdbba55f3806441f22b175557579bc0b051/actionview/lib/action_view/helpers/form_tag_helper.rb#L780) as well:

```ruby
def number_field_tag(name, value = nil, options = {})
  options = options.stringify_keys
  ...
end

[Note #1: My code uses `merge` to respect the existing behavior of
duplicating the `options` hash before updating its keys, see #17096 (comment)]

[Note #2: My code uses symbols instead of strings (e.g.: `:hidden`) to look
forward to future version of Ruby/Raiks (GC symbols); the result of the method,
however, is the same, because the symbols are stringified inside `text_field_tag`]

[Note #3: I had previously created a similar PR #17096 but decided to
split it into multiple PRs given the feedback received in the comments]
a9050e7

@sachin004 sachin004 added a commit to sachin004/rails that referenced this issue Dec 13, 2014

@claudiob @sachin004 claudiob + sachin004 Remove duplicate stringify_keys in text_field_tag
All the methods that invoke `text_field_tag` (such as `hidden_field_tag`)
and all the methods that invoke `number_field_tag` (that is `range_field_tag`)
do not need to call `stringify_keys` on their `options` parameter since the
`text_field_tag` method [is already doing it internally](https://github.com/claudiob/rails/blob/4159134524f4c78d008eef9d9a17f73a3172dcc8/actionview/lib/action_view/helpers/form_tag_helper.rb#L182):

```ruby
def text_field_tag(name, value = nil, options = {})
  tag :input, { "type" => "text", "name" => name, "id" => sanitize_to_id(name), "value" => value }.update(options.stringify_keys)
end
```

and `number_field_tag` is [already doing it internally](https://github.com/claudiob/rails/blob/e3207bdbba55f3806441f22b175557579bc0b051/actionview/lib/action_view/helpers/form_tag_helper.rb#L780) as well:

```ruby
def number_field_tag(name, value = nil, options = {})
  options = options.stringify_keys
  ...
end

[Note #1: My code uses `merge` to respect the existing behavior of
duplicating the `options` hash before updating its keys, see rails#17096 (comment)]

[Note #2: My code uses symbols instead of strings (e.g.: `:hidden`) to look
forward to future version of Ruby/Raiks (GC symbols); the result of the method,
however, is the same, because the symbols are stringified inside `text_field_tag`]

[Note #3: I had previously created a similar PR #17096 but decided to
split it into multiple PRs given the feedback received in the comments]
59d8c08

@seapy seapy added a commit to seapy/rails that referenced this issue Jan 26, 2015

@seapy seapy Merge pull request #1 from seapy/rorlakr_guides_output
Remove guides/output directory from git
9a523a0

@tenderlove tenderlove pushed a commit that referenced this issue Feb 1, 2015

@mdluo mdluo Merge pull request #1 from mdluo/pr/18316
Fix n+1 query problem when eager loading nil associations (fixes #18312)
82e710f

@keepcosmos keepcosmos pushed a commit to keepcosmos/rails that referenced this issue Jul 12, 2015

@seapy seapy Merge pull request #1 from marocchino/update-welcome
Fix repo infomation in welcome messages
83d46b4

Rails are on the railroad.

Django was a person.

@pcreux pcreux referenced this issue Oct 1, 2015

@tenderlove tenderlove remove the request parameter from `parse_formatted_parameters`
This is an instance method on the request object now so we don't need it
anymore
8db2e67

@tijmenb tijmenb added a commit to tijmenb/rails that referenced this issue Nov 3, 2015

@tijmenb tijmenb Add text template for source code
When a request is made with AJAX and an error occurs, Rails will render
a text-template for the exception instead of the HTML error page
(#11960).

The `.text.erb` variant of the `_source` template is currently missing,
causing HTML to be rendered in the response. This commit adds the text
template.

Related to #14745.

Before:

```
~/testing-exceptions  ᐅ curl 'http://localhost:3000/' -H
'X-Requested-With: XMLHttpRequest'
RuntimeError in PostsController#index

    <div class="source " id="frame-source-0">
      <div class="info">
        Extracted source (around line <strong>#3</strong>):
      </div>
      <div class="data">
        <table cellpadding="0" cellspacing="0" class="lines">
          <tr>
```

After:

```
~/testing-exceptions  ᐅ curl 'http://localhost:3000/' -H
'X-Requested-With: XMLHttpRequest'
RuntimeError in PostsController#index

Extracted source (around line #3):

#1 class PostsController < ApplicationController
#2   def index
*3     raise
#4   end
#5 end
```
054edd9

@dhh dhh pushed a commit to dhh/rails that referenced this issue Dec 14, 2015

@javan javan Merge pull request #1 from basecamp/required-channel-name
Require Cable.Channel constructors to define their channel name
2ecc9b5

@piotrj piotrj added a commit to piotrj/rails that referenced this issue Jan 30, 2016

@piotrj piotrj Update counter cache only if the relation is actually saved.
Fixes #18988, #22602, #23265

*** Description of the problem
So far the counter cached was incremented when one of two things
happened in following scenario.

```
  class Post < ActiveRecord::Base
    has_many :coments
  end

  class Comment < ActiveRecord::base
    belongs_to :post, counter_cache: true
  end
```

1. comment.post = post was called. This would increment the counter
   cache for post in the db
2. Comment was saved with updated post_id and the counter cache is
   incremented in update callback

This solution (especially the fact that we did counter cache increment
in situation #1) had few problems:

1. We bumped the counter cache even if in the end the Comment was not
   saved. For example if something like this happened:

```
    comment.post = post
    raise SomeException
```

  then the comment wouldn't have post_id, but post would end up with

2. If someone did the following
```
    comment.post = post
    comment.save!
```

   Post would end up with comments_count = 2 because the counter cache
   would get bumped on both of conditions above.

3. The bump in situation #1 was made outside of transaction that saves
the comment which means we would make unnecessary call to db.

4. comment.post = post and comment.post_id = post.id are not consistent.
Former bumps the bumper cache while the latter does not.

*** Solution
This commit removes the counter cache increment on association
assignment. This means that comment.post = post won't bump the counter.
It will only happen when the comment will get saved. This will ensure
more consistency of data in DB.

Other changes that are introduced in this commit:
1. Counter cache incrementer/decrementer can work with foreign_keys
other than the primary_key of the model. That's why methods
updates_counters_for_key, increment_counter_for_key and
decrement_counter_for_key has been introduced.

2. The update callback that bumps counter cache will properly increment
counter cache for polymorphic association even if the only thing that
was changed was type of the related object but it happened to have the
same id as the previously related object.

Notes about chaned tests:
1. In test_belongs_to_counter_with_assigning_nil I changed the models
under test as Comment's post_id is non nullable therefore comment could
never have been saved with post_id = nil. Moreover this test in its
original form shown how flawed the previous behavior of counter cache
was. When nil was assigned to comment.post, that post's comments_count
would get decremented, but in the end it would be possible to save that
comment and it would result in inconsistent data.

2. test_counter_cache_with_custom_column_name - the relation of
SillyReply is basically broken. It uses the same foreign_key "parent_id"
as the Reply (its superclass) topic relation. That's why I used
DogLovers and Dogs for the test of custom names for counter caches.

3. Other tests have been updated to actually save the objects in order
to bump counter cache.
f50543c

@piotrj piotrj added a commit to piotrj/rails that referenced this issue May 3, 2016

@piotrj piotrj Update counter cache only if the relation is actually saved.
Fixes #18988, #22602, #23265

*** Description of the problem
So far the counter cached was incremented when one of two things
happened in following scenario.

```
  class Post < ActiveRecord::Base
    has_many :coments
  end

  class Comment < ActiveRecord::base
    belongs_to :post, counter_cache: true
  end
```

1. comment.post = post was called. This would increment the counter
   cache for post in the db
2. Comment was saved with updated post_id and the counter cache is
   incremented in update callback

This solution (especially the fact that we did counter cache increment
in situation #1) had few problems:

1. We bumped the counter cache even if in the end the Comment was not
   saved. For example if something like this happened:

```
    comment.post = post
    raise SomeException
```

  then the comment wouldn't have post_id, but post would end up with

2. If someone did the following
```
    comment.post = post
    comment.save!
```

   Post would end up with comments_count = 2 because the counter cache
   would get bumped on both of conditions above.

3. The bump in situation #1 was made outside of transaction that saves
the comment which means we would make unnecessary call to db.

4. comment.post = post and comment.post_id = post.id are not consistent.
Former bumps the bumper cache while the latter does not.

*** Solution
This commit removes the counter cache increment on association
assignment. This means that comment.post = post won't bump the counter.
It will only happen when the comment will get saved. This will ensure
more consistency of data in DB.

Other changes that are introduced in this commit:
1. Counter cache incrementer/decrementer can work with foreign_keys
other than the primary_key of the model. That's why methods
updates_counters_for_key, increment_counter_for_key and
decrement_counter_for_key has been introduced.

2. The update callback that bumps counter cache will properly increment
counter cache for polymorphic association even if the only thing that
was changed was type of the related object but it happened to have the
same id as the previously related object.

Notes about chaned tests:
1. In test_belongs_to_counter_with_assigning_nil I changed the models
under test as Comment's post_id is non nullable therefore comment could
never have been saved with post_id = nil. Moreover this test in its
original form shown how flawed the previous behavior of counter cache
was. When nil was assigned to comment.post, that post's comments_count
would get decremented, but in the end it would be possible to save that
comment and it would result in inconsistent data.

2. test_counter_cache_with_custom_column_name - the relation of
SillyReply is basically broken. It uses the same foreign_key "parent_id"
as the Reply (its superclass) topic relation. That's why I used
DogLovers and Dogs for the test of custom names for counter caches.

3. Other tests have been updated to actually save the objects in order
to bump counter cache.
3672de0

Please re-open. I'm having trouble with require "Django".

irb(main):003:0> require "Django"
LoadError: cannot load such file -- Django
    from /usr/local/Cellar/ruby/2.2.3/lib/ruby/2.2.0/rubygems/core_ext/kernel_require.rb:54:in `require'
    from /usr/local/Cellar/ruby/2.2.3/lib/ruby/2.2.0/rubygems/core_ext/kernel_require.rb:54:in `require'
    from (irb):3
    from /usr/local/bin/irb:11:in `<main>'

@kaspth kaspth referenced this issue May 25, 2016

@rafaelfranca rafaelfranca Don't delegate to private methods of the targer
And make sure that it doesn't even try to call the method in the target.
3ac9956

@arthurnn arthurnn locked and limited conversation to collaborators May 30, 2016

This issue was closed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.