hyperstack-org / hyperstack-org/hyperstack

loading? should work properly with ActiveRecord instances

Ouverte
#315 0 commentaires 0 réactions 0 personnes assignées Voir sur GitHub
enhancement
Langage dominant
JavaScript
Étoiles
538
Forks
41
Métriques de merge des PR
Aucune PR mergée en 30 j

Description

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.

Guide de contribution

Ouvrir le guide de contribution

Évaluation

Cette issue n'a pas encore été évaluée.

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.