Follow Plugin 👨

Pushed this. Testing. Feel free to join me!

1 Like

Thanks, just a note but this code block is starting to look a bit unmaintainable (espec. line 162). Maybe it makes sense to extract the logic whether to include a user to a helper method, and reduce some of the code dupe? (e.g. on User.find(user_id))

def should_notify_follower(user, post)
	user.notify_me_when_followed_replies &&
		following = user.following.select { |data| data[0] == post.user_id } &&
		following.last.to_i == Follow::Notification.levels[:watching]
end

def author_replied_followers(post)
  User.find(post.user_id).followers.reduce([]) do |users, user_id|
	user = User.find(user_id)
	if user && should_notify_follower(user, post)
		users.push(user)
	else
		users
	end
  end
end

(Have not gotten a chance to test above – just an idea. I’ll test it later.)

Hold off from testing. It’s not fully resolved. Find can and will still fail. I’ll revert with another iteration.

Agree it could do with some rationalisation. But fix is more urgent.

1 Like

OK I’ve tried to rationalise and fix at the same time … let’s see how this works for you:

1 Like

btw let’s take this to PM for time being. Bit too noisy for public consumption I feel?

1 Like

Can push notification and desktop notification be received when someone follows?
@merefield

I believe this works. Email notifications almost certainly aren’t implemented.

You are welcome to PR some test cases.

2 Likes

I was trying the User Card Directory - theme - Discourse Meta and it failed to show all user card data because the /user-cards.json?user_ids=... endpoint returns an HTTP error 500.

Looking at the logs it is related to this plugin:

log

Message

StandardError (Attempted to access the non preloaded custom field ‘followers’. This is disallowed to prevent N+1 queries.)
app/models/concerns/has_custom_fields.rb:168:in []' lib/plugin/instance.rb:259:in public_send’
lib/plugin/instance.rb:259:in block (2 levels) in add_to_class' (eval):44:in _fast_attributes’
app/controllers/users_controller.rb:123:in cards' app/controllers/application_controller.rb:358:in block in with_resolved_locale’
app/controllers/application_controller.rb:358:in with_resolved_locale' lib/middleware/omniauth_bypass_middleware.rb:68:in call’
lib/content_security_policy/middleware.rb:12:in call' lib/middleware/anonymous_cache.rb:354:in call’
config/initializers/008-rack-cors.rb:25:in call' config/initializers/100-quiet_logger.rb:23:in call’
config/initializers/100-silence_logger.rb:31:in call' lib/middleware/enforce_hostname.rb:22:in call’
lib/middleware/request_tracker.rb:176:in `call’

Backtrace

app/models/concerns/has_custom_fields.rb:168:in []' plugins/discourse-follow/plugin.rb:57:in block (2 levels) in activate!’
lib/plugin/instance.rb:259:in public_send' lib/plugin/instance.rb:259:in block (2 levels) in add_to_class’
plugins/discourse-follow/plugin.rb:75:in block (2 levels) in activate!' (eval):44:in _fast_attributes’
active_model_serializers (0.8.4) lib/active_model/serializer.rb:456:in attributes' active_model_serializers (0.8.4) lib/active_model/serializer.rb:480:in _serializable_hash’
active_model_serializers (0.8.4) lib/active_model/serializer.rb:359:in serializable_hash' active_model_serializers (0.8.4) lib/active_model/array_serializer.rb:89:in block in _serializable_array’

Do you have any idea if is it possible to make them compatible?

Thanks.

Oh a stealth change to the API to accommodate a Theme Component? Sneaky! I presume that route isn’t used if you don’t install this TC?

Generally we only test on/support vanilla Discourse, but I can see a case for getting this done, not sure when though.

PR welcome.

This route is only used by the theme component? That’s odd :smile:

I could try a PR but I don’t know exactly how to fix this.

The user#cards controller is the one preloading the custom fields:

So followers is not preloaded, but it also is not used on user cards, is it? I know it doesn’t exist on card.json, which only has the following boolean. Both routes are using UserCardSerializer, but only user#cards is trying to access the followers custom field.

Currently doing a house move but if you don’t get to it first I’ll look at in when new home office is set up :).

1 Like