maverick/docbrown
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Add organizations to onboarding follow suggestions (#19564)

Browse source 
* Prepare: relocate user suggestions

* Prepare: relocate users suggestion service -> query

* Organization query for orgs with above-average scores

* Rubocop

* Limit to last 3 weeks

* Tweak recent scope, limit to 5 orgs

* Onboarding routes are also always JSON

* Divide by zero makes NaN means

* Add Orgs suggester into suggestions

* Rubocop

* select distinct orgs

* Fix for weird edge-case with bad local data

* Include type_identifier in JSON payload

* Update follows API to allow org_ids as input

* Update onboarding front-end to distinguish users/orgs

* Fix: i18n issues

* Fix: type_identifier in json output

* Fix: distinct is weird

* Fix: JS linter

* Continue tweaking front-end

* Audit import order

* Cleanup @todo note

* Try renaming controller action

* Move Article average calculation to postgres and fix math

* Refactor decorated type_identifier

* Refactor SuggestProminent, return more orgs, fix spec math

* Use FeatureFlag for organization suggestions

* This might fix the jest
This commit is contained in:
Joshua Wehner 2023-06-09 10:32:03 +02:00 • committed by GitHub
parent e573ba32d1
commit c2a131e43d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
28 changed files with 547 additions and 135 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
.rubocop.yml
View file

@ -973,3 +973,6 @@ RSpec/NoExpectationExample:
- ^expect_
- ^assert_
- sidekiq_assert_
RSpec/IndexedLet:
Enabled: false

4
app/controllers/application_controller.rb
View file

@ -287,6 +287,10 @@ class ApplicationController < ActionController::Base
Settings::General.admin_action_taken_at = Time.current # Used as cache key
end
def feature_flag_enabled?(flag_name, acting_as: current_user)
FeatureFlag.enabled?(flag_name, FeatureFlag::Actor[acting_as])
end
private
def configure_permitted_parameters

25
app/controllers/concerns/api/follows_controller.rb
View file

@ -3,11 +3,18 @@ module Api
extend ActiveSupport::Concern
def create
user_ids = params[:users].pluck("id")
user_ids.each do |user_id|
Users::FollowWorker.perform_async(current_user.id, user_id, "User")
end
render json: { outcome: I18n.t("api.v0.follows_controller.followed", count: user_ids.count) }
org_ids.each do |org_id|
Users::FollowWorker.perform_async(current_user.id, org_id, "Organization")
end
render json: {
outcome: I18n.t("api.v0.follows_controller.followed",
count: user_ids.size + org_ids.size)
}
end
def tags
@ -16,5 +23,19 @@ module Api
.includes(:followable)
.order(points: :desc)
end
private
def user_ids
return [] if params[:users].blank?
@user_ids ||= params[:users].pluck("id")
end
def org_ids
return [] if params[:organizations].blank?
@org_ids ||= params[:organizations].pluck("id")
end
end
end

18
app/controllers/onboardings_controller.rb
View file

@ -2,9 +2,10 @@ class OnboardingsController < ApplicationController
before_action :authenticate_user!
before_action :check_suspended, only: %i[notifications]
before_action :set_cache_control_headers, only: %i[show tags]
before_action :set_no_cache_header, only: %i[update]
before_action :set_no_cache_header, only: %i[update users_and_organizations]
after_action :verify_authorized, only: %i[update checkbox]
SUGGESTED_USER_ATTRIBUTES = %i[id name username summary profile_image].freeze
TAG_ONBOARDING_ATTRIBUTES = %i[id name taggings_count].freeze
ALLOWED_USER_PARAMS = %i[last_onboarding_page username].freeze
ALLOWED_CHECKBOX_PARAMS = %i[checked_code_of_conduct checked_terms_and_conditions].freeze
@ -14,6 +15,12 @@ class OnboardingsController < ApplicationController
set_surrogate_key_header "onboarding-slideshow"
end
def users_and_organizations
suggested_follows = suggested_user_follows
suggested_follows += suggested_organization_follows if feature_flag_enabled?(:suggest_organizations)
@suggestions = ApplicationDecorator.decorate_collection(suggested_follows)
end
def tags
@tags = Tags::SuggestedForOnboarding.call
.select(TAG_ONBOARDING_ATTRIBUTES)
@ -95,4 +102,13 @@ class OnboardingsController < ApplicationController
format.json { render json: { errors: errors }, status: status }
end
end
def suggested_organization_follows
Organizations::SuggestProminent.call(current_user)
end
def suggested_user_follows
Users::SuggestRecent.call(current_user,
attributes_to_select: SUGGESTED_USER_ATTRIBUTES)
end
end

26
app/controllers/users_controller.rb
View file

@ -7,19 +7,8 @@ class UsersController < ApplicationController
except: %i[index signout_confirm add_org_admin remove_org_admin remove_from_org confirm_destroy]
before_action :initialize_stripe, only: %i[edit]
INDEX_ATTRIBUTES_FOR_SERIALIZATION = %i[id name username summary profile_image].freeze
private_constant :INDEX_ATTRIBUTES_FOR_SERIALIZATION
def index
@users =
case params[:state]
when "follow_suggestions"
determine_follow_suggestions(current_user)
when "sidebar_suggestions"
Users::SuggestForSidebar.call(current_user, params[:tag]).sample(3)
else
User.none
end
@users = sidebar_suggestions || User.none
end
# GET /settings/@tab
@ -254,13 +243,6 @@ class UsersController < ApplicationController
private
def determine_follow_suggestions(current_user)
Users::SuggestRecent.call(
current_user,
attributes_to_select: INDEX_ATTRIBUTES_FOR_SERIALIZATION,
)
end
def handle_organization_tab
@organizations = @current_user.organizations.order(name: :asc)
if params[:org_id] == "new" || (params[:org_id].blank? && @organizations.empty?)
@ -323,4 +305,10 @@ class UsersController < ApplicationController
def password_params
params.permit(:current_password, :password, :password_confirmation)
end
def sidebar_suggestions
return if params[:state].to_s != "sidebar_suggestions"
Users::SuggestForSidebar.call(current_user, params[:tag]).sample(3)
end
end

4
app/decorators/application_decorator.rb
View file

@ -32,4 +32,8 @@ class ApplicationDecorator
def decorate
self
end
def type_identifier
class_name.downcase
end
end

116
app/javascript/onboarding/components/FollowUsers.jsx
View file

@ -2,8 +2,25 @@ import { h, Component } from 'preact';
import PropTypes from 'prop-types';
import he from 'he';
import { getContentOfToken } from '../utilities';
import { locale } from '../../utilities/locale';
import { Navigation } from './Navigation';
function groupFollowsByType(array) {
return array.reduce((returning, item) => {
const type = item.type_identifier
returning[type] = (returning[type] || []).concat(item);
return returning;
}, {})
}
function groupFollowIdsByType(array) {
return array.reduce((returning, item) => {
const type = item.type_identifier
returning[type] = (returning[type] || []).concat({id: item.id});
return returning;
}, {})
}
export class FollowUsers extends Component {
constructor(props) {
super(props);
@ -12,13 +29,13 @@ export class FollowUsers extends Component {
this.handleComplete = this.handleComplete.bind(this);
this.state = {
users: [],
selectedUsers: [],
follows: [],
selectedFollows: [],
};
}
componentDidMount() {
fetch('/users?state=follow_suggestions', {
fetch('/onboarding/users_and_organizations', {
headers: {
Accept: 'application/json',
'Content-Type': 'application/json',
@ -28,8 +45,8 @@ export class FollowUsers extends Component {
.then((response) => response.json())
.then((data) => {
this.setState({
selectedUsers: data,
users: data,
selectedFollows: data,
follows: data,
});
});
@ -49,8 +66,9 @@ export class FollowUsers extends Component {
handleComplete() {
const csrfToken = getContentOfToken('csrf-token');
const { selectedUsers } = this.state;
const { selectedFollows } = this.state;
const { next } = this.props;
const idsGroupedByType = groupFollowIdsByType(selectedFollows);
fetch('/api/follows', {
method: 'POST',
@ -58,7 +76,9 @@ export class FollowUsers extends Component {
'X-CSRF-Token': csrfToken,
'Content-Type': 'application/json',
},
body: JSON.stringify({ users: selectedUsers }),
body: JSON.stringify({
users: idsGroupedByType["user"],
organizations: idsGroupedByType["organization"] }),
credentials: 'same-origin',
});
@ -66,64 +86,72 @@ export class FollowUsers extends Component {
}
handleSelectAll() {
const { selectedUsers, users } = this.state;
if (selectedUsers.length === users.length) {
const { selectedFollows, follows } = this.state;
if (selectedFollows.length === follows.length) {
this.setState({
selectedUsers: [],
selectedFollows: [],
});
} else {
this.setState({
selectedUsers: users,
selectedFollows: follows,
});
}
}
handleClick(user) {
let { selectedUsers } = this.state;
handleClick(follow) {
let { selectedFollows } = this.state;
if (!selectedUsers.includes(user)) {
if (!selectedFollows.includes(follow)) {
this.setState((prevState) => ({
selectedUsers: [...prevState.selectedUsers, user],
selectedFollows: [...prevState.selectedFollows, follow],
}));
} else {
selectedUsers = [...selectedUsers];
const indexToRemove = selectedUsers.indexOf(user);
selectedUsers.splice(indexToRemove, 1);
selectedFollows = [...selectedFollows];
const indexToRemove = selectedFollows.indexOf(follow);
selectedFollows.splice(indexToRemove, 1);
this.setState({
selectedUsers,
selectedFollows,
});
}
}
renderFollowCount() {
const { users, selectedUsers } = this.state;
const { follows, selectedFollows } = this.state;
let followingStatus;
if (selectedUsers.length === 0) {
followingStatus = "You're not following anyone";
} else if (selectedUsers.length === 1) {
followingStatus = "You're following 1 person";
} else if (selectedUsers.length === users.length) {
followingStatus = `You're following ${selectedUsers.length} people (everyone) -`;
if (selectedFollows.length === 0) {
followingStatus = locale("core.not_following");
} else if (selectedFollows.length === follows.length) {
followingStatus = `${locale("core.following_everyone") }`;
} else {
followingStatus = `You're following ${selectedUsers.length} people -`;
const groups = groupFollowsByType(selectedFollows);
let together = []
for (const type in groups) {
const counted = locale(`core.counted_${type}`, {count: groups[type].length});
together = together.concat(counted)
}
const anded_together = together.join(` ${locale("core.and")} `);
followingStatus = `${locale("core.you_are_following")} ${anded_together}`;
}
const klassName =
selectedUsers.length > 0
selectedFollows.length > 0
? 'fw-bold color-base-60 inline-block fs-base'
: 'color-base-60 inline-block fs-base';
return <p className={klassName}>{followingStatus}</p>;
return <p className={klassName}>{followingStatus} -</p>;
}
renderFollowToggle() {
const { users, selectedUsers } = this.state;
const { follows, selectedFollows } = this.state;
let followText = '';
if (selectedUsers.length !== users.length) {
if (users.length === 1) {
followText = `Select ${users.length} person`;
if (selectedFollows.length !== follows.length) {
if (follows.length === 1) {
followText = `Select ${follows.length}`;
} else {
followText = `Select all ${users.length} people`;
followText = `Select all ${follows.length}`;
}
} else {
followText = 'Deselect all';
@ -141,9 +169,9 @@ export class FollowUsers extends Component {
}
render() {
const { users, selectedUsers } = this.state;
const { follows, selectedFollows } = this.state;
const { prev, slidesCount, currentSlideIndex } = this.props;
const canSkip = selectedUsers.length === 0;
const canSkip = selectedFollows.length === 0;
return (
<div
@ -178,12 +206,12 @@ export class FollowUsers extends Component {
</header>
<fieldset data-testid="onboarding-users">
{users.map((user) => {
const selected = selectedUsers.includes(user);
{follows.map((follow) => {
const selected = selectedFollows.includes(follow);
return (
<div
key={user.id}
key={`${follow.id}-${follow.type_identifier}`}
data-testid="onboarding-user-button"
className={`user content-row ${
selected ? 'selected' : 'unselected'
@ -192,15 +220,15 @@ export class FollowUsers extends Component {
<figure className="user-avatar-container">
<img
className="user-avatar"
src={user.profile_image_url}
src={follow.profile_image_url}
alt=""
loading="lazy"
/>
</figure>
<div className="user-info">
<h4 className="user-name">{user.name}</h4>
<h4 className="user-name">{follow.name}</h4>
<p className="user-summary">
{he.unescape(user.summary || '')}
{he.unescape(follow.summary || '')}
</p>
</div>
<label
@ -209,11 +237,11 @@ export class FollowUsers extends Component {
}`}
>
<input
aria-label={`Follow ${user.name}`}
aria-label={`Follow ${follow.name}`}
type="checkbox"
checked={selected}
className="absolute opacity-0 absolute top-0 bottom-0 right-0 left-0"
onClick={() => this.handleClick(user)}
onClick={() => this.handleClick(follow)}
data-testid="onboarding-user-following-status"
/>
{selected ? 'Following' : 'Follow'}

11
app/javascript/onboarding/components/__tests__/FollowUsers.test.jsx
View file

@ -4,6 +4,7 @@ import fetch from 'jest-fetch-mock';
import '@testing-library/jest-dom';
import { axe } from 'jest-axe';
import { i18nSupport } from '../../../utilities/i18n_support';
import { FollowUsers } from '../FollowUsers';
global.fetch = fetch;
@ -39,20 +40,24 @@ describe('FollowUsers', () => {
id: 1,
name: 'Ben Halpern',
profile_image_url: 'apple-icon.png',
type_identifier: 'user',
},
{
id: 2,
name: 'Krusty the Clown',
profile_image_url: 'clown.jpg',
type_identifier: 'user',
},
{
id: 3,
name: 'dev.to staff',
profile_image_url: 'dev.jpg',
type_identifier: 'user',
},
]);
beforeAll(() => {
i18nSupport();
document.head.innerHTML =
'<meta name="csrf-token" content="some-csrf-token" />';
document.body.setAttribute('data-user', getUserData());
@ -101,7 +106,7 @@ describe('FollowUsers', () => {
);
expect(queryAllByLabelText('Following')).toHaveLength(3);
expect(queryByText(/You're following 3 people \(everyone\)/i)).toExist();
expect(queryByText(/You're following everyone/i)).toExist();
expect(queryByText(/Continue/i)).toExist();
// Unfollow the first user
@ -148,13 +153,13 @@ describe('FollowUsers', () => {
expect(queryByText(/You're not following anyone/i)).toExist();
// select all then test following count
const followAllSelector = await findByText(/Select all 3 people/i);
const followAllSelector = await findByText(/Select all 3/i);
fireEvent.click(followAllSelector);
expect(queryByText('Follow')).not.toExist();
expect(queryAllByLabelText('Following')).toHaveLength(3);
expect(queryByText(/You're following 3 people \(everyone\)/i)).toExist();
expect(queryByText(/You're following everyone/i)).toExist();
});
it('should render a stepper', () => {

18
app/javascript/utilities/i18n_support.js Normal file
View file

@ -0,0 +1,18 @@
//Load the package
const fs = require('fs');
const yaml = require('js-yaml');
//Read the Yaml file
const locale = './config/locales/en.yml';
const data = fs.readFileSync(locale, 'utf8');
const yamlData = yaml.load(data);
document.body.innerHTML += `<div id="i18n-translations"></div>`;
document.getElementById('i18n-translations').dataset.translations =
JSON.stringify(yamlData);
export function i18nSupport() {
// this function doesn't really do anything
// so long as you load this module before the 'locale' utility
// the div will be there when it needs it
}

4
app/javascript/utilities/locale.js
View file

@ -12,6 +12,6 @@ const { locale: userLocale } = document.body.dataset;
if (userLocale) {
i18n.locale = userLocale;
}
export function locale(term) {
return i18n.t(term);
export function locale(term, params = {}) {
return i18n.t(term, params);
}

10
app/models/article.rb
View file

@ -388,6 +388,16 @@ class Article < ApplicationRecord
scope :eager_load_serialized_data, -> { includes(:user, :organization, :tags) }
scope :above_average, lambda {
order(:score).where("score >= ?", average_score)
}
def self.average_score
Rails.cache.fetch("article_average_score", expires_in: 1.day) do
unscoped { where(score: 0..).average(:score) } || 0.0
end
end
def self.seo_boostable(tag = nil, time_ago = 18.days.ago)
# Time ago sometimes returns this phrase instead of a date
time_ago = 5.days.ago if time_ago == "latest"

14
app/models/tag.rb
View file

@ -170,6 +170,20 @@ class Tag < ActsAsTaggableOn::Tag
.order(hotness_score: :desc)
end
def self.followed_by(user, points = (1..))
where(
id: Follow.where(
follower_id: user.id,
followable_type: "ActsAsTaggableOn::Tag",
points: points,
).select(:followable_id),
)
end
def self.antifollowed_by(user)
followed_by(user, (...1))
end
# @return [String]
#
# @see ApplicationRecord#class_name

22
app/models/user.rb
View file

@ -324,31 +324,17 @@ class User < ApplicationRecord
true
end
# @todo Move the Query logic into Tag. It represents User understanding the inner working of Tag.
def cached_followed_tag_names
cache_name = "user-#{id}-#{following_tags_count}-#{last_followed_at&.rfc3339}/followed_tag_names"
Rails.cache.fetch(cache_name, expires_in: 24.hours) do
Tag.where(
id: Follow.where(
follower_id: id,
followable_type: "ActsAsTaggableOn::Tag",
points: 1..,
).select(:followable_id),
).pluck(:name)
Tag.followed_by(self).pluck(:name)
end
end
# @todo Move the Query logic into Tag. It represents User understanding the inner working of Tag.
def cached_antifollowed_tag_names
cache_name = "user-#{id}-#{following_tags_count}-#{last_followed_at&.rfc3339}/antifollowed_tag_names"
Rails.cache.fetch(cache_name, expires_in: 24.hours) do
Tag.where(
id: Follow.where(
follower_id: id,
followable_type: "ActsAsTaggableOn::Tag",
points: ...1,
).select(:followable_id),
).pluck(:name)
Tag.antifollowed_by(self).pluck(:name)
end
end
@ -556,6 +542,10 @@ class User < ApplicationRecord
last_moderation_notification, last_notification_activity].compact.max
end
def currently_following_tags
Tag.followed_by(self)
end
protected
# Send emails asynchronously

69
app/queries/organizations/suggest_prominent.rb Normal file
View file

@ -0,0 +1,69 @@
module Organizations
class SuggestProminent
class_attribute :average_score
class_attribute :article_finder, default: Article
RECENTLY = 3
MAX = 5
def self.recently_published_at
(RECENTLY.weeks.ago..)
end
def self.call(...)
new(...).suggest
end
def initialize(current_user)
@current_user = current_user
end
def suggest
scope = article_finder.merge(above_average_articles)
.merge(organization_affiliated_articles)
.merge(recently_published_articles)
from_followed = scope.merge(articles_from_followed_tags)
# Prefer orgs from followed tags, fill from the larger scope if needed
# Postgres doesn't like when DISTINCT doesn't line up with SELECT,
# which means doing this in the DB would require a sub-select.
# UNION would also be an option, except that UNION does not guarantee
# result order. If we want followed-tags-orgs to come earlier in the
# list, this seems to be our best option.
unique_org_ids = unique_org_ids(from_followed)
if unique_org_ids.size <= MAX
additional_org_ids = unique_org_ids(scope)[..(MAX - unique_org_ids.size)]
unique_org_ids += additional_org_ids
end
Organization.where(id: unique_org_ids).limit(MAX)
end
private
attr_reader :current_user
def above_average_articles
article_finder.above_average
end
def organization_affiliated_articles
article_finder.where.not(organization_id: nil)
end
def recently_published_articles
article_finder.where(published_at: self.class.recently_published_at)
end
def articles_from_followed_tags
tags = current_user.currently_following_tags.pluck(:name).join(",")
return article_finder.none if tags.blank?
article_finder.tagged_with(tags, any: true)
end
def unique_org_ids(scoped_query)
scoped_query.pluck(:organization_id).uniq
end
end
end

0
app/services/users/suggest_recent.rb → app/queries/users/suggest_recent.rb
View file

7
app/views/onboardings/users_and_organizations.json.jbuilder Normal file
View file

@ -0,0 +1,7 @@
json.array! @suggestions.each do |suggested|
json.extract!(suggested, :id, :name, :username, :type_identifier)
json.summary truncate(suggested.tag_line || t("json.author", community: community_name), length: 100)
json.profile_image_url suggested.profile_image_url_for(length: 90)
json.following false
end

10
config/locales/en.yml
View file

@ -19,6 +19,16 @@ en:
report_abuse: Report abuse
search: Search
success_settings: Successfully updated settings.
counted_organization:
one: "%{count} organization"
other: "%{count} organizations"
counted_user:
one: "%{count} person"
other: "%{count} people"
not_following: "You're not following anyone"
following_everyone: "You're following everyone"
you_are_following: "You're following"
and: "and"
datetime:
distance_in_words_ago: # https://github.com/openstreetmap/openstreetmap-website/issues/2255
about_x_hours:

10
config/locales/fr.yml
View file

@ -19,6 +19,16 @@ fr:
report_abuse: Signaler un abus
search: Recherche
success_settings: Successfully updated settings.
counted_organization:
one: "%{count} organisation"
other: "%{count} organisations"
counted_user:
one: "%{count} personne"
other: "%{count} personnes"
not_following: "Vous ne suivez personne"
following_everyone: "Vous les suivez tous"
you_are_following: "Vous suivez"
and: "et"
date:
abbr_month_names:
-

7
config/routes.rb
View file

@ -165,9 +165,10 @@ Rails.application.routes.draw do
resource :onboarding, only: %i[show update] do
member do
patch :checkbox
patch :notifications
get :tags
patch :checkbox, defaults: { format: :json }
patch :notifications, defaults: { format: :json }
get :tags, defaults: { format: :json }
get :users_and_organizations, defaults: { format: :json }
end
end

13
spec/decorators/application_decorator_spec.rb
View file

@ -54,4 +54,17 @@ RSpec.describe ApplicationDecorator, type: :decorator do
expect(decorated_collection.map(&:object)).to eq(relation.to_a)
end
end
describe ".type_identifier" do
it "returns the class downcased" do
decorated_article = Article.new.decorate
expect(decorated_article.type_identifier).to eq("article")
decorated_user = User.new.decorate
expect(decorated_user.type_identifier).to eq("user")
decorated_organization = Organization.new.decorate
expect(decorated_organization.type_identifier).to eq("organization")
end
end
end

30
spec/models/article_spec.rb
View file

@ -700,8 +700,8 @@ RSpec.describe Article do
describe "#slug" do
let(:title) { "hey This' is$ a SLUG" }
let(:article0) { build(:article, title: title, published: false) } # rubocop:disable RSpec/IndexedLet
let(:article1) { build(:article, title: title, published: false) } # rubocop:disable RSpec/IndexedLet
let(:article0) { build(:article, title: title, published: false) }
let(:article1) { build(:article, title: title, published: false) }
before do
article0.validate!
@ -1360,6 +1360,32 @@ RSpec.describe Article do
end
end
describe ".above_average and .average_score" do
context "when there are not yet any articles with score above 0" do
it "works as expected" do
expect(described_class.average_score).to be_within(0.1).of(0.0)
articles = described_class.above_average
expect(articles.pluck(:score)).to contain_exactly(0)
end
end
context "when there are articles with score" do
before do
create(:article, score: 10)
create(:article, score: 6)
create(:article, score: 4)
create(:article, score: 1)
# averages 4.2 with article created earlier, see let on line 13
end
it "works as expected" do
expect(described_class.average_score).to be_within(0.1).of(4.2)
articles = described_class.above_average
expect(articles.pluck(:score)).to contain_exactly(10, 6)
end
end
end
it "does not send moderator notifications when a draft post" do
allow(Notification).to receive(:send_moderation_notification)

25
spec/models/tag_spec.rb
View file

@ -212,6 +212,31 @@ RSpec.describe Tag do
end
end
describe "followed_by and antifollowed_by" do
let(:user) { create(:user) }
let(:follow_tag) { create(:tag, name: "following") }
let(:antifollow_tag) { create(:tag, name: "antifollowing") }
let(:unrelated) { create(:tag, name: "unrelated") }
let(:other) { create(:user) }
before do
follow = user.follow(follow_tag)
follow.update explicit_points: 5
antifollow = user.follow(antifollow_tag)
antifollow.update explicit_points: -5
other.follow(unrelated)
end
it "works as expected" do
results = described_class.followed_by(user)
expect(results).to contain_exactly(follow_tag)
antiresults = described_class.antifollowed_by(user)
expect(antiresults).to contain_exactly(antifollow_tag)
end
end
# [@jeremyf] The implementation details of #accessible_name are contingent on a feature flag that
# we're using for this refactoring. Once we remove the flag, please adjust the specs
# accordingly.

11
spec/models/user_spec.rb
View file

@ -854,4 +854,15 @@ RSpec.describe User do
expect(results).to contain_exactly(later)
end
end
describe "#currently_following_tags" do
before do
allow(Tag).to receive(:followed_by)
end
it "calls Tag.followed_by" do
user.currently_following_tags
expect(Tag).to have_received(:followed_by).with(user)
end
end
end

68
spec/queries/organizations/suggest_prominent_spec.rb Normal file
View file

@ -0,0 +1,68 @@
require "rails_helper"
RSpec.describe Organizations::SuggestProminent, type: :service do
subject(:suggester) { described_class.new(current_user) }
let(:current_user) { create(:user) }
let(:top_organizations) { create_list(:organization, 4) }
let(:bad_organizations) { create_list(:organization, 2) }
let(:mid_included) { create(:organization) }
let(:mid_excluded) { create(:organization) }
before do
top_organizations.each do |organization|
create_list(:article, 3, organization_id: organization.id, score: 15)
end
bad_organizations.each do |organization|
create(:article, organization_id: organization.id, score: 1)
end
create(:article, organization_id: mid_included.id, score: 15)
create(:article, organization_id: mid_excluded.id, score: 14)
end
context "when user is following any tags" do
let(:followed) { create(:tag) }
let(:unfollowed) { create(:tag) }
let(:suggest_this_org) { create(:organization) }
let(:dont_suggest_this) { create(:organization) }
before do
current_user.follow(followed)
create_list(:article, 3, organization_id: suggest_this_org.id, score: 25, tags: followed.name)
create(:article, organization_id: dont_suggest_this.id, score: 15, tags: unfollowed.name)
end
it "returns organizations with posts with at least an average score under followed tags" do
results = suggester.suggest
expect(results).not_to be_blank
expect(results).to include(suggest_this_org)
expect(results).not_to include(dont_suggest_this)
expect(results).not_to include(bad_organizations.first)
expect(results).not_to include(bad_organizations.last)
end
end
it "returns max 5 organizations with posts with at least an average score under any tags" do
results = suggester.suggest
expect(results.size).to eq(5)
expect(results).to include(*top_organizations)
expect(results).to include(mid_included)
expect(results).not_to include(mid_excluded)
expect(results).not_to include(bad_organizations.first)
expect(results).not_to include(bad_organizations.last)
end
it "does not include orgs with posts published more than 3 weeks ago" do
old_timer = nil # so this is still accessible after block
Timecop.travel(5.weeks.ago) do
old_timer = create(:organization)
create(:article, organization_id: old_timer.id, score: 50)
end
results = suggester.suggest
expect(results).not_to include(old_timer)
end
end

0
spec/services/users/suggest_recent_spec.rb → spec/queries/users/suggest_recent_spec.rb
View file

12
spec/requests/api/v1/follows_spec.rb
View file

@ -12,14 +12,22 @@ RSpec.describe "Api::V1::FollowsController" do
context "when user is authorized" do
let(:user) { create(:user) }
let(:users_hash) { [{ id: create(:user).id }, { id: create(:user).id }] }
let(:orgs_hash) { [{ id: create(:organization).id }, { id: create(:organization).id }] }
before do
sign_in user
end
it "returns the number of followed users" do
post "/api/follows", params: { users: users_hash }, headers: headers
expect(response.parsed_body["outcome"]).to include("#{users_hash.size} users")
post "/api/follows",
params: {
users: users_hash,
organizations: orgs_hash
},
headers: headers
expected_outcome = users_hash.size + orgs_hash.size
expect(response.parsed_body["outcome"]).to include("#{expected_outcome} users")
end
it "creates follows" do

104
spec/requests/onboardings_spec.rb
View file

@ -40,6 +40,110 @@ RSpec.describe "Onboardings" do
end
end
describe "GET /users_and_organizations" do
context "when no suggestions are found" do
it "returns an empty array (no automated suggested follow)" do
sign_in user
get users_and_organizations_onboarding_path
expect(response).to have_http_status(:ok)
expect(response.parsed_body).to eq([])
end
end
context "when only user suggestions are found" do
let(:tag) { create(:tag) }
let(:other_user) { create(:user) }
# Prepare auto-generated user suggestions
before do
user.follow(tag)
create(:article, user: other_user, tags: [tag.name])
end
it "returns follow suggestions for an authenticated user" do
sign_in user
get users_and_organizations_onboarding_path
response_user = response.parsed_body.first
expect(response_user["id"]).to eq(other_user.id)
end
it "returns follow suggestions that have profile images" do
sign_in user
get users_and_organizations_onboarding_path
response_user = response.parsed_body.first
expect(response_user["profile_image_url"]).to eq(other_user.profile_image_url)
end
end
context "when organization suggestions are found" do
let(:suggested_orgs) { create_list(:organization, 2) }
let(:expected_json_keys) do
%w[id name username profile_image_url following summary type_identifier]
end
before do
allow(FeatureFlag).to receive(:enabled?).and_return(true)
allow(Organizations::SuggestProminent).to receive(:call).and_return(suggested_orgs)
end
it "returns organization follow suggestions for an authenticated user" do
sign_in user
get users_and_organizations_onboarding_path
response_org_ids = response.parsed_body.pluck("id")
expect(response_org_ids.size).to eq(2)
suggested_org_ids = suggested_orgs.map(&:id)
expect(response_org_ids).to match_array(suggested_org_ids)
end
it "returns organization follow suggestions that have profile images" do
sign_in user
get users_and_organizations_onboarding_path
response_org = response.parsed_body.first
expect(response_org.keys).to match_array(expected_json_keys)
expect(response_org["profile_image_url"]).to eq(suggested_orgs.first.profile_image_url)
end
end
context "when organization and user suggestions are found" do
let(:suggested_users) { create_list(:user, 2) }
let(:suggested_orgs) { create_list(:organization, 2) }
let(:expected_json_keys) do
%w[id name username profile_image_url following summary type_identifier]
end
before do
allow(FeatureFlag).to receive(:enabled?).and_return(true)
allow(Organizations::SuggestProminent).to receive(:call).and_return(suggested_orgs)
allow(Users::SuggestRecent).to receive(:call).and_return(suggested_users)
end
it "returns users first, then organizations" do
sign_in user
get users_and_organizations_onboarding_path
response_ids = response.parsed_body.pluck("id")
expect(response_ids.size).to eq(4)
suggested_ids = suggested_users.map(&:id) + suggested_orgs.map(&:id)
expect(response_ids).to eq(suggested_ids)
end
end
end
describe "GET /onboarding/tags" do
let(:headers) do
{

41
spec/requests/user/user_suggestions_spec.rb
View file

@ -14,47 +14,6 @@ RSpec.describe "Users" do
end
end
context "when follow_suggestions params are present and no suggestions are found" do
it "returns an empty array (no automated suggested follow)" do
sign_in user
get users_path(state: "follow_suggestions")
expect(response).to have_http_status(:ok)
expect(response.parsed_body).to eq([])
end
end
context "when follow_suggestions params are present" do
let(:user) { create(:user) }
let(:tag) { create(:tag) }
let(:other_user) { create(:user) }
before do
# Prepare auto-generated user suggestions
user.follow(tag)
create(:article, user: other_user, tags: [tag.name])
end
it "returns follow suggestions for an authenticated user" do
sign_in user
get users_path(state: "follow_suggestions")
response_user = response.parsed_body.first
expect(response_user["id"]).to eq(other_user.id)
end
it "returns follow suggestions that have profile images" do
sign_in user
get users_path(state: "follow_suggestions")
response_user = response.parsed_body.first
expect(response_user["profile_image_url"]).to eq(other_user.profile_image_url)
end
end
context "when sidebar_suggestions params are present" do
it "returns no sidebar suggestions for an authenticated user" do
sign_in create(:user)