Some topics and synapses were hidden from users erroneously (#944)

* ensure topics and synapses have their permission match the map they're deferring to

* update permission of topics and synapses as map perm changes, when defer_to_map

* try enabling count threshold on rubocop

* remove unused mk_permission functions

* change *_count methods to use delegate to save lines in map.rb model

* rubocop topic.rb
This commit is contained in:
Connor Turland 2016-12-06 16:46:46 -05:00 committed by GitHub
parent d2074ada79
commit a133702be2
11 changed files with 38 additions and 59 deletions

View file

@ -8,6 +8,7 @@ engines:
enabled: true
config:
languages:
count_threshold: 3 # rule of three
ruby:
mass_threshold: 36 # default: 18
javascript:

View file

@ -32,14 +32,19 @@ class Map < ApplicationRecord
# Validate the attached image is image/jpg, image/png, etc
validates_attachment_content_type :screenshot, content_type: /\Aimage\/.*\Z/
after_save :update_deferring_topics_and_synapses, if: :permission_changed?
delegate :count, to: :topics, prefix: :topic # same as `def topic_count; topics.count; end`
delegate :count, to: :synapses, prefix: :synapse
delegate :count, to: :contributors, prefix: :contributor
delegate :count, to: :stars, prefix: :star
delegate :name, to: :user, prefix: true
def mappings
topicmappings.or(synapsemappings)
end
def mk_permission
Perm.short(permission)
end
def contributors
User.where(id: mappings.map(&:user_id).uniq)
end
@ -48,28 +53,10 @@ class Map < ApplicationRecord
User.where(id: user_id).or(User.where(id: collaborators))
end
def topic_count
topics.length
end
def synapse_count
synapses.length
end
delegate :name, to: :user, prefix: true
def user_image
user.image.url(:thirtytwo)
end
def contributor_count
contributors.length
end
def star_count
stars.length
end
def collaborator_ids
collaborators.map(&:id)
end
@ -131,4 +118,9 @@ class Map < ApplicationRecord
end
removed.compact
end
def update_deferring_topics_and_synapses
Topic.where(defer_to_map_id: id).update_all(permission: permission)
Synapse.where(defer_to_map_id: id).update_all(permission: permission)
end
end

View file

@ -36,11 +36,7 @@ class Synapse < ApplicationRecord
end
end
def calculated_permission
defer_to_map&.permission || permission
end
def as_json(_options = {})
super(methods: [:user_name, :user_image, :calculated_permission, :collaborator_ids])
super(methods: [:user_name, :user_image, :collaborator_ids])
end
end

View file

@ -75,12 +75,8 @@ class Topic < ApplicationRecord
Pundit.policy_scope(user, maps).map(&:id)
end
def calculated_permission
defer_to_map&.permission || permission
end
def as_json(options = {})
super(methods: [:user_name, :user_image, :calculated_permission, :collaborator_ids])
super(methods: [:user_name, :user_image, :collaborator_ids])
.merge(inmaps: inmaps(options[:user]), inmapsLinks: inmapsLinks(options[:user]),
map_count: map_count(options[:user]), synapse_count: synapse_count(options[:user]))
end
@ -129,15 +125,14 @@ class Topic < ApplicationRecord
"Get: #{name}"
end
def mk_permission
Perm.short(permission)
end
protected
def create_metamap?
if link == '' and metacode.name == 'Metamap'
@map = Map.create({ name: name, permission: permission, desc: '', arranged: true, user_id: user_id })
self.link = Rails.application.routes.url_helpers.map_url(:host => ENV['MAILER_DEFAULT_URL'], :id => @map.id)
end
end
def create_metamap?
return unless (link == '') && (metacode.name == 'Metamap')
@map = Map.create(name: name, permission: permission, desc: '',
arranged: true, user_id: user_id)
self.link = Rails.application.routes.url_helpers
.map_url(host: ENV['MAILER_DEFAULT_URL'], id: @map.id)
end
end

View file

@ -2,11 +2,10 @@
class SynapsePolicy < ApplicationPolicy
class Scope < Scope
def resolve
visible = %w(public commons)
return scope.where(permission: visible) unless user
return scope.where(permission: %w(public commons)) unless user
scope.where(permission: visible)
.or(scope.where.not(defer_to_map_id: nil).where(defer_to_map_id: user.all_accessible_maps.map(&:id)))
scope.where(permission: %w(public commons))
.or(scope.where(defer_to_map_id: user.all_accessible_maps.map(&:id)))
.or(scope.where(user_id: user.id))
end
end

View file

@ -2,11 +2,10 @@
class TopicPolicy < ApplicationPolicy
class Scope < Scope
def resolve
visible = %w(public commons)
return scope.where(permission: visible) unless user
return scope.where(permission: %w(public commons)) unless user
scope.where(permission: visible)
.or(scope.where.not(defer_to_map_id: nil).where(defer_to_map_id: user.all_accessible_maps.map(&:id)))
scope.where(permission: %w(public commons))
.or(scope.where(defer_to_map_id: user.all_accessible_maps.map(&:id)))
.or(scope.where(user_id: user.id))
end
end

View file

@ -38,7 +38,6 @@ const Synapse = Backbone.Model.extend({
newOptions.success = function(model, response, opt) {
if (s) s(model, response, opt)
model.set('calculated_permission', model.get('permission'))
model.trigger('saved')
if (permBefore === 'private' && model.get('permission') !== 'private') {
@ -85,7 +84,7 @@ const Synapse = Backbone.Model.extend({
</li>`
},
authorizeToEdit: function(mapper) {
if (mapper && (this.get('calculated_permission') === 'commons' || this.get('collaborator_ids').includes(mapper.get('id')) || this.get('user_id') === mapper.get('id'))) return true
if (mapper && (this.get('permission') === 'commons' || this.get('collaborator_ids').includes(mapper.get('id')) || this.get('user_id') === mapper.get('id'))) return true
else return false
},
authorizePermissionChange: function(mapper) {

View file

@ -37,7 +37,6 @@ const Topic = Backbone.Model.extend({
newOptions.success = function(model, response, opt) {
if (s) s(model, response, opt)
model.set('calculated_permission', model.get('permission'))
model.trigger('saved')
if (permBefore === 'private' && model.get('permission') !== 'private') {
@ -82,7 +81,7 @@ const Topic = Backbone.Model.extend({
authorizeToEdit: function(mapper) {
if (mapper &&
(this.get('user_id') === mapper.get('id') ||
this.get('calculated_permission') === 'commons' ||
this.get('permission') === 'commons' ||
this.get('collaborator_ids').includes(mapper.get('id')))) {
return true
} else {

View file

@ -295,8 +295,7 @@ const Import = {
permission: topicPermision,
defer_to_map_id: deferToMapId,
desc: desc || '',
link: link || '',
calculated_permission: Active.Map.get('permission')
link: link || ''
})
DataModel.Topics.add(topic)

View file

@ -174,7 +174,7 @@ const SynapseCard = {
add_perms_form: function(synapse) {
// permissions - if owner, also allow permission editing
$('#editSynLowerBar').append('<div class="mapPerm ' + synapse.get('calculated_permission').substring(0, 2) + '"></div>')
$('#editSynLowerBar').append('<div class="mapPerm ' + synapse.get('permission').substring(0, 2) + '"></div>')
// ability to change permission
var selectingPermission = false

View file

@ -448,8 +448,8 @@ const TopicCard = {
nodeValues.inmaps += '<li class="hideExtra extraText"><a href="' + url + '">' + inmapsAr[i] + '</a></li>'
}
}
nodeValues.permission = topic.get('calculated_permission')
nodeValues.mk_permission = topic.get('calculated_permission').substring(0, 2)
nodeValues.permission = topic.get('permission')
nodeValues.mk_permission = topic.get('permission').substring(0, 2)
nodeValues.map_count = topic.get('map_count').toString()
nodeValues.synapse_count = topic.get('synapse_count').toString()
nodeValues.id = topic.isNew() ? topic.cid : topic.id