hyperstack-org / hyperstack-org/hyperstack

loading? should work properly with ActiveRecord instances

Đang mở
#315 0 bình luận 0 reaction 0 người được giao Xem trên GitHub
enhancement
Ngôn ngữ chính
JavaScript
Star
538
Fork
41
Chỉ số merge pull request
Không có pull request nào được merge trong 30 ngày

Mô tả

If `foo` is some instance of an activerecord model then `foo.loading?` should return true if *any* of the attributes in foo are loading.

Right now `loading?` only works with attributes, but not the entire record, no on collections of records

```ruby
module ActiveRecord
module InstanceMethods

def loading?
to_check = [self]
next_check = 0
while next_check < to_check.length
to_check[next_check].attributes.values.each do |value|
if value.is_a? ActiveRecord::Base
to_check << value unless to_check.include? value
elsif value.loading?
puts "loading is true"
return true
end
end
next_check += 1
end
false
end

def loaded?
!loading?
end
end
```

Likewise for collections... but to make this more efficient you probably want the base method to be something like array_loading?(array) -> returns true/false, using the above algorithm, but initializing the array from the param.

That way the collection can call array_loading and not worry about re checking records inside the tree more than once.

BUT the other big catch is that the `set_attribute_change_status_and_notify` internal method needs to change so that it only checks no active record models.

```ruby
module ReactiveRecord
class Base # was module Setters
def attribute_loaded?(current_value)
current_value.is_a?(ActiveRecord::Base) || current_value.loaded?
end

def set_attribute_change_status_and_notify(attr, changed, new_value)
if @virgin
@attributes[attr] = new_value
else
change_status_and_notify_helper(attr, changed) do |had_key, current_value|
@attributes[attr] = new_value
if !data_loading? || # following line was current_value.loaded?
(on_opal_client? && had_key && attribute_loaded?(current_value) && current_value != new_value)
Hyperstack::Internal::State::Variable.set(self, attr, new_value, data_loading?)
end
end
end
end
end
end
```

This change is the right thing to do, but is going to be a bit problematic for existing HS systems, as people have probably come to depend on some_model.loading? working the way it does today.

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.