Fix for AO3-4580 - inviting to hidden/anon collection causes work to become hidden/anon #2468
3 participants
houndci-bot
commented on an outdated diff
lib/collectible.rb
| def update_anon_unrevealed | ||
| if self.respond_to?(:in_anon_collection) && self.respond_to?(:in_unrevealed_collection) | ||
| - self.in_anon_collection = !self.collection_items.select {|c| c.anonymous? }.empty? | ||
| - self.in_unrevealed_collection = !self.collection_items.select{|c| c.unrevealed? }.empty? | ||
| + self.in_anon_collection = !self.user_approved_collection_items.select {|c| c.anonymous? }.empty? | ||
| + self.in_unrevealed_collection = !self.user_approved_collection_items.select{|c| c.unrevealed? }.empty? |
|
Redundant
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
houndci-bot
commented on an outdated diff
lib/collectible.rb
| def update_anon_unrevealed | ||
| if self.respond_to?(:in_anon_collection) && self.respond_to?(:in_unrevealed_collection) | ||
| - self.in_anon_collection = !self.collection_items.select {|c| c.anonymous? }.empty? | ||
| - self.in_unrevealed_collection = !self.collection_items.select{|c| c.unrevealed? }.empty? | ||
| + self.in_anon_collection = !self.user_approved_collection_items.select {|c| c.anonymous? }.empty? |
|
Redundant
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
houndci-bot
commented on an outdated diff
lib/collectible.rb
| def set_anon_unrevealed | ||
| if self.respond_to?(:in_anon_collection) && self.respond_to?(:in_unrevealed_collection) | ||
| - self.in_anon_collection = !self.collections.select {|c| c.anonymous? }.empty? | ||
| - self.in_unrevealed_collection = !self.collections.select{|c| c.unrevealed? }.empty? | ||
| + self.in_anon_collection = !self.user_approved_collections.select {|c| c.anonymous? }.empty? | ||
| + self.in_unrevealed_collection = !self.user_approved_collections.select{|c| c.unrevealed? }.empty? |
|
Redundant
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
houndci-bot
commented on an outdated diff
lib/collectible.rb
| @@ -16,6 +18,10 @@ def self.included(collectible) | ||
| :through => :collection_items, | ||
| :source => :collection, | ||
| :conditions => ['collection_items.user_approval_status = ? AND collection_items.collection_approval_status = ?', CollectionItem::APPROVED, CollectionItem::APPROVED] | ||
| + has_many :user_approved_collections, | ||
| + :through => :collection_items, | ||
| + :source => :collection, | ||
| + :conditions => ['collection_items.user_approval_status = ?', CollectionItem::APPROVED] |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
houndci-bot
commented on an outdated diff
lib/collectible.rb
| def set_anon_unrevealed | ||
| if self.respond_to?(:in_anon_collection) && self.respond_to?(:in_unrevealed_collection) | ||
| - self.in_anon_collection = !self.collections.select {|c| c.anonymous? }.empty? | ||
| - self.in_unrevealed_collection = !self.collections.select{|c| c.unrevealed? }.empty? | ||
| + self.in_anon_collection = !self.user_approved_collections.select {|c| c.anonymous? }.empty? |
|
Redundant
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
houndci-bot
commented on an outdated diff
lib/collectible.rb
| @@ -16,6 +18,10 @@ def self.included(collectible) | ||
| :through => :collection_items, | ||
| :source => :collection, | ||
| :conditions => ['collection_items.user_approval_status = ? AND collection_items.collection_approval_status = ?', CollectionItem::APPROVED, CollectionItem::APPROVED] | ||
| + has_many :user_approved_collections, | ||
| + :through => :collection_items, | ||
| + :source => :collection, |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
houndci-bot
commented on an outdated diff
lib/collectible.rb
| @@ -7,6 +7,8 @@ def self.included(collectible) | ||
| accepts_nested_attributes_for :collection_items, :allow_destroy => true | ||
| has_many :approved_collection_items, :class_name => "CollectionItem", :as => :item, | ||
| :conditions => ['collection_items.user_approval_status = ? AND collection_items.collection_approval_status = ?', CollectionItem::APPROVED, CollectionItem::APPROVED] | ||
| + has_many :user_approved_collection_items, :class_name => "CollectionItem", :as => :item, | ||
| + :conditions => ['collection_items.user_approval_status = ?', CollectionItem::APPROVED] |
|
Align the elements of a hash literal if they span more than one line.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
houndci-bot
commented on an outdated diff
lib/collectible.rb
| @@ -16,6 +18,10 @@ def self.included(collectible) | ||
| :through => :collection_items, | ||
| :source => :collection, | ||
| :conditions => ['collection_items.user_approval_status = ? AND collection_items.collection_approval_status = ?', CollectionItem::APPROVED, CollectionItem::APPROVED] | ||
| + has_many :user_approved_collections, | ||
| + :through => :collection_items, |
|
Align the parameters of a method call if they span more than one line.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
houndci-bot
commented on an outdated diff
lib/collectible.rb
| @@ -7,6 +7,8 @@ def self.included(collectible) | ||
| accepts_nested_attributes_for :collection_items, :allow_destroy => true | ||
| has_many :approved_collection_items, :class_name => "CollectionItem", :as => :item, | ||
| :conditions => ['collection_items.user_approval_status = ? AND collection_items.collection_approval_status = ?', CollectionItem::APPROVED, CollectionItem::APPROVED] | ||
| + has_many :user_approved_collection_items, :class_name => "CollectionItem", :as => :item, |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
lib/collectible.rb
| @@ -3,23 +3,29 @@ module Collectible | ||
| def self.included(collectible) | ||
| collectible.class_eval do | ||
| - has_many :collection_items, :as => :item, :dependent => :destroy, :inverse_of => :item | ||
| - accepts_nested_attributes_for :collection_items, :allow_destroy => true | ||
| - has_many :approved_collection_items, :class_name => "CollectionItem", :as => :item, | ||
| - :conditions => ['collection_items.user_approval_status = ? AND collection_items.collection_approval_status = ?', CollectionItem::APPROVED, CollectionItem::APPROVED] | ||
| + has_many :collection_items, as: :item, dependent: :destroy, inverse_of: :item | ||
| + accepts_nested_attributes_for :collection_items, allow_destroy: true | ||
| + has_many :approved_collection_items, class_name: "CollectionItem", as: :item, | ||
| + conditions: ['collection_items.user_approval_status = ? AND collection_items.collection_approval_status = ?', CollectionItem::APPROVED, CollectionItem::APPROVED] | ||
| + has_many :user_approved_collection_items, class_name: "CollectionItem", as: :item, | ||
| + conditions: ['collection_items.user_approval_status = ?', CollectionItem::APPROVED] |
|
Align the elements of a hash literal if they span more than one line.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
lib/collectible.rb
| def update_anon_unrevealed | ||
| if self.respond_to?(:in_anon_collection) && self.respond_to?(:in_unrevealed_collection) | ||
| - self.in_anon_collection = !self.collection_items.select {|c| c.anonymous? }.empty? | ||
| - self.in_unrevealed_collection = !self.collection_items.select{|c| c.unrevealed? }.empty? | ||
| + self.in_anon_collection = !self.user_approved_collection_items.select(&:anonymous?).empty? |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
houndci-bot
commented on an outdated diff
lib/collectible.rb
| def set_anon_unrevealed | ||
| if self.respond_to?(:in_anon_collection) && self.respond_to?(:in_unrevealed_collection) | ||
| - self.in_anon_collection = !self.collections.select {|c| c.anonymous? }.empty? | ||
| - self.in_unrevealed_collection = !self.collections.select{|c| c.unrevealed? }.empty? | ||
| + self.in_anon_collection = !self.user_approved_collections.select(&:anonymous?).empty? | ||
| + self.in_unrevealed_collection = !self.user_approved_collections.select(&:unrevealed?).empty? |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
houndci-bot
commented on an outdated diff
lib/collectible.rb
| def set_anon_unrevealed | ||
| if self.respond_to?(:in_anon_collection) && self.respond_to?(:in_unrevealed_collection) | ||
| - self.in_anon_collection = !self.collections.select {|c| c.anonymous? }.empty? | ||
| - self.in_unrevealed_collection = !self.collections.select{|c| c.unrevealed? }.empty? | ||
| + self.in_anon_collection = !self.user_approved_collections.select(&:anonymous?).empty? |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
lib/collectible.rb
| def update_anon_unrevealed | ||
| if self.respond_to?(:in_anon_collection) && self.respond_to?(:in_unrevealed_collection) | ||
| - self.in_anon_collection = !self.collection_items.select {|c| c.anonymous? }.empty? | ||
| - self.in_unrevealed_collection = !self.collection_items.select{|c| c.unrevealed? }.empty? | ||
| + self.in_anon_collection = !self.user_approved_collection_items.select(&:anonymous?).empty? | ||
| + self.in_unrevealed_collection = !self.user_approved_collection_items.select(&:unrevealed?).empty? |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
lib/collectible.rb
| @@ -3,23 +3,29 @@ module Collectible | ||
| def self.included(collectible) | ||
| collectible.class_eval do | ||
| - has_many :collection_items, :as => :item, :dependent => :destroy, :inverse_of => :item | ||
| - accepts_nested_attributes_for :collection_items, :allow_destroy => true | ||
| - has_many :approved_collection_items, :class_name => "CollectionItem", :as => :item, | ||
| - :conditions => ['collection_items.user_approval_status = ? AND collection_items.collection_approval_status = ?', CollectionItem::APPROVED, CollectionItem::APPROVED] | ||
| + has_many :collection_items, as: :item, dependent: :destroy, inverse_of: :item | ||
| + accepts_nested_attributes_for :collection_items, allow_destroy: true | ||
| + has_many :approved_collection_items, class_name: "CollectionItem", as: :item, | ||
| + conditions: ['collection_items.user_approval_status = ? AND collection_items.collection_approval_status = ?', CollectionItem::APPROVED, CollectionItem::APPROVED] |
|
Align the elements of a hash literal if they span more than one line.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
lib/collectible.rb
| if c.closed? | ||
| - errors.add(:base, ts("The collection %{name} is not currently open.", :name => name)) and return unless c.user_is_maintainer?(User.current_user) || old_collections.include?(c.id) | ||
| + errors.add(:base, ts("The collection %{name} is not currently open.", name: name)) and return unless c.user_is_maintainer?(User.current_user) || old_collections.include?(c.id) |
|
Non-local exit from iterator, without return value.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
lib/collectible.rb
| @@ -29,9 +35,9 @@ def collections_to_add=(collection_names) | ||
| names = trim_collection_names(collection_names) | ||
| names.each do |name| | ||
| c = Collection.find_by_name(name) | ||
| - errors.add(:base, ts("We couldn't find the collection %{name}.", :name => name)) and return if c.nil? | ||
| + errors.add(:base, ts("We couldn't find the collection %{name}.", name: name)) and return if c.nil? |
|
Non-local exit from iterator, without return value.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
I am unfortunately not sure why this is failing, but marking Needs Coder Action.
Yep a bunch of the tests need fixing because they expect the behavior we don't want (ie unapproved by user work being hidden/anon), can fix them Tuesday
lib/collectible.rb
| def set_anon_unrevealed | ||
| if self.respond_to?(:in_anon_collection) && self.respond_to?(:in_unrevealed_collection) | ||
| - self.in_anon_collection = !self.collections.select {|c| c.anonymous? }.empty? | ||
| - self.in_unrevealed_collection = !self.collections.select{|c| c.unrevealed? }.empty? | ||
| + # if we have collection items saved here then the collectible is not a new object | ||
| + if self.collection_items.empty? |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
2 checks passed
Details
continuous-integration/travis-ci/pr
The Travis CI build passed
hound
9 violations found.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Fixes https://otwarchive.atlassian.net/browse/AO3-4580
Signed-off-by: astolat shalott [email protected]