Skip to content

Commit

Permalink
Resolves #1319 - Link TODOs to recorded issues
Browse files Browse the repository at this point in the history
  • Loading branch information
botimer committed Sep 19, 2018
1 parent fe9ca1c commit e2efc3f
Show file tree
Hide file tree
Showing 16 changed files with 22 additions and 2 deletions.
1 change: 1 addition & 0 deletions lib/active_fedora/aggregation/list_source.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ def persist_ordered_self
# Set node subjects to a term in AF JUST so that AF will persist the
# sub-graphs.
# TODO: Find a way to fix this.
# See https://github.com/samvera/active_fedora/issues/1337
resource.set_value(:nodes, [])
self.nodes += graph.subjects.to_a
ordered_self.changes_committed!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ def self.define_callback(model, callback_name, name, options)
full_callback_name = "#{callback_name}_for_#{name}"

# TODO : why do i need method_defined? I think its because of the inheritance chain
# See https://github.com/samvera/active_fedora/issues/1333
model.class_attribute full_callback_name.to_sym unless model.method_defined?(full_callback_name)

callbacks = Array(options[callback_name.to_sym]).map do |callback|
Expand Down
3 changes: 3 additions & 0 deletions lib/active_fedora/associations/collection_association.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ def ids_writer(ids)
ids = Array(ids).reject(&:blank?)
replace(klass.find(ids)) # .index_by { |r| r.id }.values_at(*ids))
# TODO, like this when find() can return multiple records
# See https://github.com/samvera/active_fedora/issues/1340
# send("#{reflection.name}=", reflection.klass.find(ids))
# send("#{reflection.name}=", ids.collect { |id| reflection.klass.find(id)})
end
Expand Down Expand Up @@ -171,6 +172,7 @@ def concat_records(*records)
# See delete for more info.
def delete_all
# TODO: load_target causes extra loads. Can't we just send delete requests?
# See https://github.com/samvera/active_fedora/issues/1341
delete(load_target).tap do
reset
loaded!
Expand Down Expand Up @@ -301,6 +303,7 @@ def construct_query
def find_target
# TODO: don't reify, just store the solr results and lazily reify.
# For now, we set a hard limit of 1000 results.
# See https://github.com/samvera/active_fedora/issues/1358
records = ActiveFedora::QueryResultBuilder.reify_solr_results(load_from_solr(rows: SolrService::MAX_ROWS))
records.each { |record| set_inverse_instance(record) }
records
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ module Associations
class DirectlyContainsOneAssociation < SingularAssociation #:nodoc:
# Finds objects contained by the container predicate (either the configured has_member_relation or ldp:contains)
# TODO: Refactor this to use solr (for efficiency) instead of parsing the RDF graph. Requires indexing ActiveFedora::File objects into solr, including their RDF.type and, if possible, the id of their container
# See https://github.com/samvera/active_fedora/issues/1335
def find_target
# filtered_objects = container_association_proxy.to_a.select { |o| o.metadata_node.type.include?(options[:type]) }
query_node = if container_predicate = options[:has_member_relation]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ module ActiveFedora
module Associations
# TODO: we may want to split this into two subclasses, one for has_member_relation
# and the other for is_member_of_relation
# See https://github.com/samvera/active_fedora/issues/1332
class IndirectlyContainsAssociation < ContainsAssociation #:nodoc:
# Add +records+ to this association. Returns +self+ so method calls may be chained.
# Since << flattens its argument list and inserts each record, +push+ and +concat+ behave identically.
Expand Down Expand Up @@ -40,6 +41,7 @@ def find_target
uris.map { |object_uri| klass.find(klass.uri_to_id(object_uri)) }
else # is_member_of_relation
# TODO this is a lot of reads. Avoid this path
# See https://github.com/samvera/active_fedora/issues/1345
container_predicate = ::RDF::Vocab::LDP.contains
proxy_uris = container.resource.query(predicate: container_predicate).map { |r| r.object.to_s }
proxy_uris.map { |uri| proxy_class.find(proxy_class.uri_to_id(uri))[options[:foreign_key]] }
Expand Down
1 change: 1 addition & 0 deletions lib/active_fedora/associations/singular_association.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ def build(attributes = {})

def find_target
# TODO: this forces a solr query, but I think it's likely we can just lookup from Fedora.
# See https://github.com/samvera/active_fedora/issues/1330
rec = scope.take
rec.tap { |record| set_inverse_instance(record) }
end
Expand Down
1 change: 1 addition & 0 deletions lib/active_fedora/attributes/property_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ def #{name}=(value)

def build(&block)
# TODO: remove this block stuff
# See https://github.com/samvera/active_fedora/issues/1336
NodeConfig.new(name, options[:predicate], options.except(:predicate)) do |config|
config.with_index(&block) if block_given?
end
Expand Down
2 changes: 2 additions & 0 deletions lib/active_fedora/indexing/field_mapper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ def set_field(doc, name, value, *indexer_args)
# @api
# Given a field name, index_type, etc., returns the corresponding Solr name.
# TODO field type is the input format, maybe we could just detect that?
# See https://github.com/samvera/active_fedora/issues/1338
# @param [String] field_name the ruby (term) name which will get a suffix appended to become a Solr field name
# @param opts - index_type is only needed if the FieldDescriptor requires it (e.g. :searcahble)
# @return [String] name of the solr field, based on the params
Expand Down Expand Up @@ -69,6 +70,7 @@ def solr_names_and_values(field_name, field_value, index_types)

# Is there a custom converter?
# TODO instead of a custom converter, look for input data type and output data type. Create a few methods that can do that cast.
# See https://github.com/samvera/active_fedora/issues/1339

value = if converter
if converter.arity == 1
Expand Down
4 changes: 3 additions & 1 deletion lib/active_fedora/indexing/suffix.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,14 @@ def to_s
end

def self.config
# TODO: `:symbol' usage ought to be deprecated
# See https://github.com/samvera/active_fedora/issues/1334
@config ||= OpenStruct.new fields: [:type, :stored, :indexed, :multivalued],
suffix_delimiter: '_',
type_suffix: (lambda do |fields|
type = fields.first
case type
when :string, :symbol # TODO: `:symbol' usage ought to be deprecated
when :string, :symbol
's'
when :text
't'
Expand Down
1 change: 1 addition & 0 deletions lib/active_fedora/loadable_from_json.rb
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ def init_with_json(json)
@resource = SolrBackedResource.new(self.class)
self.attributes = adapt_attributes(attrs)
# TODO: Should we clear the change tracking, or make this object Read-only?
# See https://github.com/samvera/active_fedora/issues/1342

yield self if block_given?

Expand Down
1 change: 1 addition & 0 deletions lib/active_fedora/nested_attributes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ def accepts_nested_attributes_for(*attr_names)
reflection.autosave = true
define_autosave_association_callbacks(reflection)
## TODO this ought to work, but doesn't seem to do the class inheritance right
# See https://github.com/samvera/active_fedora/issues/1343

nested_attributes_options = self.nested_attributes_options.dup
nested_attributes_options[association_name.to_sym] = options
Expand Down
2 changes: 1 addition & 1 deletion lib/active_fedora/reflection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ def check_validity_of_inverse!
def collect_join_chain
[self]
end
alias chain collect_join_chain # todo
alias chain collect_join_chain # TODO Remove alias, See https://github.com/samvera/active_fedora/issues/1347

def has_inverse?
inverse_name
Expand Down
1 change: 1 addition & 0 deletions lib/active_fedora/relation/finder_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ def condition_to_clauses(key, value, join_with = SolrQueryBuilder.default_join_w
def field_name_for(key)
if @klass.delegated_attributes.key?(key)
# TODO: Check to see if `key' is a possible solr field for this class, if it isn't try :searchable instead
# See https://github.com/samvera/active_fedora/issues/1344
ActiveFedora.index_field_mapper.solr_name(key, :stored_searchable, type: :string)
elsif key == :id
ActiveFedora.id_field
Expand Down
1 change: 1 addition & 0 deletions lib/active_fedora/relation/merger.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ def initialize(relation, other)

def merge
# TODO: merge order
# See https://github.com/samvera/active_fedora/issues/1329
relation.where_values += other.where_values
relation
end
Expand Down
1 change: 1 addition & 0 deletions lib/active_fedora/with_metadata.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ def metadata_node

def create_or_update(*)
return unless super && !new_record?
# TODOs captured as https://github.com/samvera/active_fedora/issues/1331
metadata_node.metadata_uri = described_by # TODO: only necessary if the URI was < > before
metadata_node.save # TODO if changed?
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
1.4: default auto-phrase (QueryParser feature) to off
1.5: omitNorms defaults to true for primitive field types (int, float, boolean, string...)
# TODO 1.6: useDocValuesAsStored defaults to true.
# See https://github.com/samvera/active_fedora/issues/1346
-->

<types>
Expand Down

0 comments on commit e2efc3f

Please sign in to comment.