Add subscriptions to the dashboard

[atsmith813]

What type of PR is this? (check all applicable)

• Feature

Description

This PR add subscriptions to the dashboard. Big shoutout to @ludwiczakpawel from #9052 💯 .

I'll copy and paste this description on each PR.
We are building a liquid tag where permitted authors can embed a button that users/readers can click to share the email address associated with their DEV account with the author.

We are restricting this liquid tag only for use in Articles (not Comments or Pages) and for use only by approved organizations for CodeLand (using a new role).

• Create a new database table to hold subscriptions
• Capture the subscriber's email address
• Create a new endpoint to handle email signups from the new liquid tag
• Create a user subscription button liquid tag
• Add subscriptions to the dashbaord
• Create a new role for users approved to use the new user subscription liquid tag and add docs

Related Tickets & Documents

Closes #8169

QA Instructions, Screenshots, Recordings

1. Make sure you're signed in locally as an admin user.
2. Write a post and include the liquid tag {% user_subscription Some sweet CTA text %}.
3. Preview and publish your Article.
4. View your Article.
5. Subscribe to yourself (allowing this behavior is intentional).
6. Go to your dashboard.
7. You should see subscriptions on the article you just created and subscribed to - click that.
8. See your subscriptions 🎉 .

The data icon shows the number of subscriptions

You can also see subscriptions in the dropdown menu

Subscriptions page

Accessing a route directly for an article without any subscriptions

Added tests?

• yes

Added to documentation?

• no documentation needed

[atsmith813]

I'll need an admin override for Codeclimate.

[atsmith813]

Paging isn't technically needed yet, but I figured I may as well 🤷‍♂️

[atsmith813]

This can easily be extended to allow organization admins to see subscriptions for articles they didn't write themselves in the future.

[atsmith813]

This won't quite work if/when we extend subscriptions to other objects, like comments, in the future (Comment doesn't have a title). We can cross that bridge when we get there. It's a quick update. We also don't have the concept of a "comment dashboard" so it's possible that view would live somewhere else anyways.

[mstruve]

Couple small performance suggestions otherwise this is looking good!

[mstruve]

This is going to issue a db call every time we call it which is twice, here and below on line 98. We should cache the value and then apply it in each place. OR if we wanted to take our db calls from 3 to 1, we could just do article.user_subscriptions.size and then for the booleans check if size is greater than 0

[atsmith813]

Darn, I thought .any? was smart enough to check it in memory since we eagerly loaded them already 🤔 . I'll cache it 😢 .

[atsmith813]

@mstruve Updated to use counter_culture 🎉 Thanks for catching this!

[rhymes]

for the rationale behind any?, .present?, .size and the other methods: 3 ActiveRecord Mistakes That Slow Down Rails Apps: Count, Where and Present

unfortunately the whole deal is quite complicated :D

[mstruve]

One suggestion, we have had some issues with flakiness around roles in specs, one way to avoid this would be to stub the role so user.admin? returns true rather than adding the role itself. This would also help speed up the specs. Just a thought.

[atsmith813]

The roles are going to change in the next PR, so I'll address it there 👍 .

[mstruve]

I don't think we need to add a whole new column for this, I just meant cacheing it in an instance variable in the controller that we could then use it in the views.

[atsmith813]

We need the count on each article for this view. What does that look like with instance variables?

[atsmith813]

Bullet doesn't like us using .size and throws a message about using a counter cache instead.

[mstruve]

I think that makes sense if you are calling .size all over the place but in this instance we all need it in one place so in the controller I would do

@article_sub_count = article.subscriptions.size

Then in the views use that instance variable.

[mstruve]

@atsmith813 and I chatted a bit about the pros and cons of adding a column vs locally caching this on the article. I have a hard time with adding a column that will be minimally used(not many articles will have it) and is only to show a nice to have stat. In the end, users are subscribing to users, not articles so this number is less a must have and more a nice to have. Adding a whole column to an already bloated model for a nice to have doesnt sit well with me. On the flip side it does eliminate the need for a query.

Interested to hear other's opinions!

[atsmith813]

Going to chime in here with some additional context @mstruve. I'm also interested to hear how others feel!

TLDR - I'm thinking what we have here is the move and the challenge (issue?) is a symptom of something else that we can address separately in another PR after a team discussion.

so this number is less a must have and more a nice to have.

In this same view, we use the same setup as this PR is now trying to do using the DB to cache counts. It's what we do in other controllers. It's how the counter_culture gem and ActiveRecord's counter_cache (which we aren't using right now) work. In all areas, the counts are stored on the records so an additional query isn't needed. Maybe part of the issue is we are using the DB for these cached counts which causes us to have to add a column in the first place 🤔 .

The counts are there for other items in this view (page views, comments, reactions, etc.) and they use the same approach. It is, in some ways, a nice to have the count of subscriptions. It's also consistent with other areas of this view.

In the end, users are subscribing to users, not articles

I don't think that's the case anymore. That's one way of looking at it. If we look back at what we have now that's not as true as it was when we first discussed this. If it were purely about a user to user, we would have a view where an author just saw a list of emails/subscribers, regardless of where the subscription came from. It would also mean that the uniqueness of a subscription would be enforced strictly by user IDs. Neither of those is how it works. We show the subscriptions by Article (source) and a user can subscribe to the same author via multiple articles. So for all intents and purposes, it's actually setup more to be that users are subscribing to articles. When it comes to delivering the UX to both readers and authors, the context/source of the subscription always matters.

Adding a whole column to an already bloated model

In other words, if we were taking this approach on a different model that wasn't so bloated, would it be okay?

Either way, it feels like the issue is more about the Article model and that counting subscriptions is a symptom of this issue. It doesn't feel great that we would be considering breaking convention, ignore the warnings from Bullet, and code up a custom exception here. This issue will inevitably arise again when we want to efficiently count something else related to Articles, which are at the crux of what we do.

It would feel better if the issue of a bloated model, especially as it relates to the counting of relationship records, was addressed separately. In the case of this PR, we are very slightly adding to an existing problem by adding another column for counts. When we figure out a gameplan to address this separately, this implementation would get swept up with the counts of comments, page views, reactions, and many other columns.

I understand there's a cost and a tradeoff that's growing with each column we keep adding to Article and that cost doesn't feel worth it for something used that will currently be used so little. Since this is for Codeland, this will be used by major organizations, and it's following convention, I'm hoping we can stick to it here.

[mstruve]

Neither of those is how it works. We show the subscriptions by Article (source) and a user can subscribe to the same author via multiple articles.

When did that change? And what is the purpose of subscribing to a user multiple times? Would people make different email lists based on articles? If that is the case we might want to make sure its explicit that if you sign up via one article you may end up on a different email list than another.

[atsmith813]

I should clarify - it's less of a change and I think more of a misunderstanding and miscommunication.

Would people make different email lists based on articles?

Correct! An author could write an article about Rails, gather subscriptions, and email those subscribers relevant content. The same author could write another article about React, gather subscriptions, and email those subscribers relevant and different content. The author may have a "one time ask" related to an event, a survey, etc.

That's where the call-to-action text by the author comes in handy. It tells the user why they're clicking "subscribe". Something like, "want to learn more about Rails? Subscribe below to be included in our weekly updates about the top Rails tips!".

The purpose is for a user to express intent/interest to a specific call-to-action and for an author to be able to "segment" their subscribers enabling both sides with more power to be connected to more relevant content.

In the event an author doesn't include any call-to-action text, we make it clear to the user what they're doing and what they're sharing.

Technically, going back to the example above of an author writing about Rails and React, there's nothing stopping that author from gathering all subscribers from both lists and emailing them the same content, regardless of relevancy. Once the subscription is made, the rest of the interaction between the author and the subscriber moves outside of our platform. The author would be subject to the "standards of email marketing" like giving the user a way to unsubscribe from them.

[lisasy]

From a design standpoint, this LGTM 🚢

[ludwiczakpawel]

Yeap, looks good design-wise. Not going to accept since @lisasy has already done it for design and I can see there's an ongoing debate around some backend-related issue so I'd rather prefer someone else to give another accept so it doesn't get merged by mistake before all issues are resolved.

[benhalpern]

I like all this and like the use of counter caches.

[juliannatetreault]

This is sweet! 🚀

[vaidehijoshi]

Overall, works great! I two small questions :)