From 431179dd257d2412fcdd8212d22dac2e491231b6 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Mon, 6 Jun 2016 16:58:35 -0400 Subject: [PATCH] FEATURE: Prompt users when they are entering duplicate links --- .../components/composer-editor.js.es6 | 1 + .../components/composer-message.js.es6 | 2 +- .../components/composer-messages.js.es6 | 28 +++++++++---- .../discourse/components/d-editor.js.es6 | 1 - .../discourse/controllers/composer.js.es6 | 39 ++++++++++++++++++- .../discourse/lib/link-lookup.js.es6 | 24 ++++++++++++ .../discourse/templates/composer.hbs | 5 ++- .../templates/composer/custom-body.hbs | 2 + app/assets/stylesheets/desktop/compose.scss | 7 ++++ .../composer_messages_controller.rb | 9 ++++- app/models/topic_link.rb | 21 ++++++++++ config/locales/client.en.yml | 1 + lib/composer_messages_finder.rb | 16 ++++---- .../composer_messages_finder_spec.rb | 34 ++++++++-------- .../composer_messages_controller_spec.rb | 2 +- spec/fabricators/post_fabricator.rb | 1 + spec/models/topic_link_spec.rb | 17 +++++++- 17 files changed, 168 insertions(+), 42 deletions(-) create mode 100644 app/assets/javascripts/discourse/lib/link-lookup.js.es6 create mode 100644 app/assets/javascripts/discourse/templates/composer/custom-body.hbs diff --git a/app/assets/javascripts/discourse/components/composer-editor.js.es6 b/app/assets/javascripts/discourse/components/composer-editor.js.es6 index 16f341e4f..76d89193c 100644 --- a/app/assets/javascripts/discourse/components/composer-editor.js.es6 +++ b/app/assets/javascripts/discourse/components/composer-editor.js.es6 @@ -459,6 +459,7 @@ export default Ember.Component.extend({ // Paint oneboxes $('a.onebox', $preview).each((i, e) => Discourse.Onebox.load(e, refresh)); this.trigger('previewRefreshed', $preview); + this.sendAction('afterRefresh', $preview); }, } }); diff --git a/app/assets/javascripts/discourse/components/composer-message.js.es6 b/app/assets/javascripts/discourse/components/composer-message.js.es6 index 74c11864e..5f8697d7f 100644 --- a/app/assets/javascripts/discourse/components/composer-message.js.es6 +++ b/app/assets/javascripts/discourse/components/composer-message.js.es6 @@ -5,7 +5,7 @@ export default Ember.Component.extend({ @computed('message.templateName') defaultLayout(templateName) { - return this.container.lookup(`template:${templateName}`) + return this.container.lookup(`template:composer/${templateName}`) }, didInsertElement() { diff --git a/app/assets/javascripts/discourse/components/composer-messages.js.es6 b/app/assets/javascripts/discourse/components/composer-messages.js.es6 index 8c343940e..ce933a5f8 100644 --- a/app/assets/javascripts/discourse/components/composer-messages.js.es6 +++ b/app/assets/javascripts/discourse/components/composer-messages.js.es6 @@ -1,3 +1,5 @@ +import LinkLookup from 'discourse/lib/link-lookup'; + export default Ember.Component.extend({ classNameBindings: [':composer-popup-container', 'hidden'], checkedMessages: false, @@ -17,6 +19,7 @@ export default Ember.Component.extend({ this.appEvents.on('composer:opened', this, this._findMessages); this.appEvents.on('composer:find-similar', this, this._findSimilar); this.appEvents.on('composer-messages:close', this, this._closeTop); + this.appEvents.on('composer-messages:create', this, this._create); }, willDestroyElement() { @@ -24,6 +27,7 @@ export default Ember.Component.extend({ this.appEvents.off('composer:opened', this, this._findMessages); this.appEvents.off('composer:find-similar', this, this._findSimilar); this.appEvents.off('composer-messages:close', this, this._closeTop); + this.appEvents.off('composer-messages:create', this, this._create); }, _closeTop() { @@ -82,20 +86,21 @@ export default Ember.Component.extend({ this.get('queuedForTyping').forEach(msg => this.send("popup", msg)); }, + _create(info) { + this.reset(); + this.send('popup', Ember.Object.create(info)); + }, + groupsMentioned(groups) { // reset existing messages, this should always win it is critical this.reset(); groups.forEach(group => { - const msg = I18n.t('composer.group_mentioned', { + const body = I18n.t('composer.group_mentioned', { group: "@" + group.name, count: group.user_count, group_link: Discourse.getURL(`/group/${group.name}/members`) }); - this.send("popup", - Em.Object.create({ - templateName: 'composer/group-mentioned', - body: msg}) - ); + this.send("popup", Ember.Object.create({ templateName: 'custom-body', body })); }); }, @@ -123,7 +128,7 @@ export default Ember.Component.extend({ const similarTopics = this.get('similarTopics'); const message = this._similarTopicsMessage || composer.store.createRecord('composer-message', { id: 'similar_topics', - templateName: 'composer/similar-topics', + templateName: 'similar-topics', extraClass: 'similar-topics' }); @@ -147,7 +152,7 @@ export default Ember.Component.extend({ if (this.get('checkedMessages')) { return; } const composer = this.get('composer'); - const args = { composerAction: composer.get('action') }; + const args = { composer_action: composer.get('action') }; const topicId = composer.get('topic.id'); const postId = composer.get('post.id'); @@ -156,6 +161,13 @@ export default Ember.Component.extend({ const queuedForTyping = this.get('queuedForTyping'); composer.store.find('composer-message', args).then(messages => { + + // Checking composer messages on replies can give us a list of links to check for + // duplicates + if (messages.extras && messages.extras.duplicate_lookup) { + this.sendAction('addLinkLookup', new LinkLookup(messages.extras.duplicate_lookup)); + } + this.set('checkedMessages', true); messages.forEach(msg => msg.wait_for_typing ? queuedForTyping.addObject(msg) : this.send('popup', msg)); }); diff --git a/app/assets/javascripts/discourse/components/d-editor.js.es6 b/app/assets/javascripts/discourse/components/d-editor.js.es6 index 01afbacd9..9a4382a86 100644 --- a/app/assets/javascripts/discourse/components/d-editor.js.es6 +++ b/app/assets/javascripts/discourse/components/d-editor.js.es6 @@ -561,5 +561,4 @@ export default Ember.Component.extend({ }); } } - }); diff --git a/app/assets/javascripts/discourse/controllers/composer.js.es6 b/app/assets/javascripts/discourse/controllers/composer.js.es6 index 2cf69458a..a4ececaa9 100644 --- a/app/assets/javascripts/discourse/controllers/composer.js.es6 +++ b/app/assets/javascripts/discourse/controllers/composer.js.es6 @@ -3,6 +3,7 @@ import Quote from 'discourse/lib/quote'; import Draft from 'discourse/models/draft'; import Composer from 'discourse/models/composer'; import { default as computed, observes } from 'ember-addons/ember-computed-decorators'; +import { relativeAge } from 'discourse/lib/formatter'; function loadDraft(store, opts) { opts = opts || {}; @@ -46,7 +47,6 @@ export default Ember.Controller.extend({ replyAsNewTopicDraft: Em.computed.equal('model.draftKey', Composer.REPLY_AS_NEW_TOPIC_KEY), checkedMessages: false, messageCount: null, - showEditReason: false, editReason: null, scopedCategoryId: null, @@ -54,6 +54,8 @@ export default Ember.Controller.extend({ lastValidatedAt: null, isUploading: false, topic: null, + linkLookup: null, + showToolbar: Em.computed({ get(){ const keyValueStore = this.container.lookup('key-value-store:main'); @@ -104,6 +106,39 @@ export default Ember.Controller.extend({ }.property('model.creatingPrivateMessage', 'model.targetUsernames'), actions: { + addLinkLookup(linkLookup) { + this.set('linkLookup', linkLookup); + }, + + afterRefresh($preview) { + const linkLookup = this.get('linkLookup'); + if (linkLookup) { + const $links = $('a[href]', $preview); + + $links.each((idx, l) => { + const href = $(l).prop('href'); + if (href && href.length) { + const [warn, info] = linkLookup.check(href); + + if (warn) { + const body = I18n.t('composer.duplicate_link', { + domain: info.domain, + username: info.username, + ago: relativeAge(new Date(info.posted_at), { format: 'medium' }), + href + }); + this.appEvents.trigger('composer-messages:create', { + extraClass: 'custom-body', + templateName: 'custom-body', + body + }); + return false; + } + } + return true; + }); + } + }, toggleWhisper() { this.toggleProperty('model.whisper'); @@ -435,6 +470,8 @@ export default Ember.Controller.extend({ // Given a potential instance and options, set the model for this composer. _setModel(composerModel, opts) { + this.set('linkList', null); + if (opts.draft) { composerModel = loadDraft(this.store, opts); if (composerModel) { diff --git a/app/assets/javascripts/discourse/lib/link-lookup.js.es6 b/app/assets/javascripts/discourse/lib/link-lookup.js.es6 new file mode 100644 index 000000000..04451ffc8 --- /dev/null +++ b/app/assets/javascripts/discourse/lib/link-lookup.js.es6 @@ -0,0 +1,24 @@ +const _warned = {}; + +export default class LinkLookup { + + constructor(links) { + this._links = links; + } + + check(href) { + if (_warned[href]) { return [false, null]; } + + const normalized = href.replace(/^https?:\/\//, ''); + if (_warned[normalized]) { return [false, null]; } + + const linkInfo = this._links[normalized]; + if (linkInfo) { + _warned[href] = true; + _warned[normalized] = true; + return [true, linkInfo]; + } + + return [false, null]; + } +}; diff --git a/app/assets/javascripts/discourse/templates/composer.hbs b/app/assets/javascripts/discourse/templates/composer.hbs index b5422104c..9bf668db7 100644 --- a/app/assets/javascripts/discourse/templates/composer.hbs +++ b/app/assets/javascripts/discourse/templates/composer.hbs @@ -9,7 +9,7 @@ {{/popup-menu}} {{/if}} - {{composer-messages composer=model messageCount=messageCount}} + {{composer-messages composer=model messageCount=messageCount addLinkLookup="addLinkLookup"}}
{{#if site.mobileView}} @@ -92,7 +92,8 @@ importQuote="importQuote" showOptions="showOptions" showToolbar=showToolbar - showUploadSelector="showUploadSelector"}} + showUploadSelector="showUploadSelector" + afterRefresh="afterRefresh"}} {{#if currentUser}}
diff --git a/app/assets/javascripts/discourse/templates/composer/custom-body.hbs b/app/assets/javascripts/discourse/templates/composer/custom-body.hbs new file mode 100644 index 000000000..07abacc36 --- /dev/null +++ b/app/assets/javascripts/discourse/templates/composer/custom-body.hbs @@ -0,0 +1,2 @@ +{{fa-icon "close"}} +

{{{message.body}}}

diff --git a/app/assets/stylesheets/desktop/compose.scss b/app/assets/stylesheets/desktop/compose.scss index 9cf3948f8..7137ef9db 100644 --- a/app/assets/stylesheets/desktop/compose.scss +++ b/app/assets/stylesheets/desktop/compose.scss @@ -78,6 +78,13 @@ } } +.custom-body { + background-color: dark-light-diff($tertiary, $secondary, 85%, -65%); + p { + max-width: 98%; + } +} + .similar-topics { background-color: dark-light-diff($tertiary, $secondary, 85%, -65%); diff --git a/app/controllers/composer_messages_controller.rb b/app/controllers/composer_messages_controller.rb index 51ce6d4c2..57bebbf8d 100644 --- a/app/controllers/composer_messages_controller.rb +++ b/app/controllers/composer_messages_controller.rb @@ -5,9 +5,16 @@ class ComposerMessagesController < ApplicationController before_filter :ensure_logged_in def index - finder = ComposerMessagesFinder.new(current_user, params.slice(:composerAction, :topic_id, :post_id)) + finder = ComposerMessagesFinder.new(current_user, params.slice(:composer_action, :topic_id, :post_id)) json = { composer_messages: [finder.find].compact } + if params[:composer_action] == "reply" && params[:topic_id].present? + topic = Topic.where(id: params[:topic_id]).first + if guardian.can_see?(topic) + json[:extras] = {duplicate_lookup: TopicLink.duplicate_lookup(topic)} + end + end + render_json_dump(json, rest_serializer: true) end end diff --git a/app/models/topic_link.rb b/app/models/topic_link.rb index 193ccbc8c..c1e54cd4c 100644 --- a/app/models/topic_link.rb +++ b/app/models/topic_link.rb @@ -214,6 +214,27 @@ class TopicLink < ActiveRecord::Base def crawl_link_title Jobs.enqueue(:crawl_topic_link, topic_link_id: id) end + + def self.duplicate_lookup(topic) + builder = SqlBuilder.new("SELECT tl.url, tl.domain, u.username_lower, p.created_at + FROM topic_links AS tl + INNER JOIN posts AS p ON p.id = tl.post_id + INNER JOIN users AS u ON p.user_id = u.id + /*where*/ + ORDER BY p.created_at DESC + LIMIT 200") + + builder.where('tl.topic_id = :topic_id', topic_id: topic.id) + + lookup = {} + + builder.exec.to_a.each do |row| + normalized = row['url'].downcase.sub(/^https?:\/\//, '') + lookup[normalized] = {domain: row['domain'], username: row['username_lower'], posted_at: row['created_at']} + end + + lookup + end end # == Schema Information diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 6d98a70db..4ee08c4f3 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -989,6 +989,7 @@ en: drafts_offline: "drafts offline" group_mentioned: "By mentioning {{group}}, you are about to notify {{count}} people." + duplicate_link: "It looks like your link to {{domain}} was already posted in the topic by @{{username}} in an earlier reply on {{ago}}" error: title_missing: "Title is required" diff --git a/lib/composer_messages_finder.rb b/lib/composer_messages_finder.rb index 68d13189d..86f03680d 100644 --- a/lib/composer_messages_finder.rb +++ b/lib/composer_messages_finder.rb @@ -29,7 +29,7 @@ class ComposerMessagesFinder education_posts_text = I18n.t('education.until_posts', count: SiteSetting.educate_until_posts) return { id: 'education', - templateName: 'composer/education', + templateName: 'education', wait_for_typing: true, body: PrettyText.cook(I18n.t(education_key, education_posts_text: education_posts_text, site_name: SiteSetting.title)) } @@ -44,7 +44,7 @@ class ComposerMessagesFinder { id: 'too_many_replies', - templateName: 'composer/education', + templateName: 'education', body: PrettyText.cook(I18n.t('education.too_many_replies', newuser_max_replies_per_topic: SiteSetting.newuser_max_replies_per_topic)) } end @@ -70,7 +70,7 @@ class ComposerMessagesFinder # Return the message { id: 'avatar', - templateName: 'composer/education', + templateName: 'education', body: PrettyText.cook(I18n.t('education.avatar', profile_path: "/users/#{@user.username_lower}")) } end @@ -108,7 +108,7 @@ class ComposerMessagesFinder { id: 'sequential_replies', - templateName: 'composer/education', + templateName: 'education', wait_for_typing: true, extraClass: 'education-message', body: PrettyText.cook(I18n.t('education.sequential_replies')) @@ -140,7 +140,7 @@ class ComposerMessagesFinder { id: 'dominating_topic', - templateName: 'composer/education', + templateName: 'education', wait_for_typing: true, extraClass: 'education-message', body: PrettyText.cook(I18n.t('education.dominating_topic', percent: (ratio * 100).round)) @@ -156,7 +156,7 @@ class ComposerMessagesFinder { id: 'reviving_old', - templateName: 'composer/education', + templateName: 'education', wait_for_typing: false, extraClass: 'education-message', body: PrettyText.cook(I18n.t('education.reviving_old_topic', days: (Time.zone.now - @topic.last_posted_at).round / 1.day)) @@ -166,11 +166,11 @@ class ComposerMessagesFinder private def creating_topic? - @details[:composerAction] == "createTopic" + @details[:composer_action] == "createTopic" end def replying? - @details[:composerAction] == "reply" + @details[:composer_action] == "reply" end end diff --git a/spec/components/composer_messages_finder_spec.rb b/spec/components/composer_messages_finder_spec.rb index 9e7c20c4c..d4e0aaedc 100644 --- a/spec/components/composer_messages_finder_spec.rb +++ b/spec/components/composer_messages_finder_spec.rb @@ -6,7 +6,7 @@ describe ComposerMessagesFinder do context "delegates work" do let(:user) { Fabricate.build(:user) } - let(:finder) { ComposerMessagesFinder.new(user, composerAction: 'createTopic') } + let(:finder) { ComposerMessagesFinder.new(user, composer_action: 'createTopic') } it "calls all the message finders" do finder.expects(:check_education_message).once @@ -24,7 +24,7 @@ describe ComposerMessagesFinder do let(:user) { Fabricate.build(:user) } context 'creating topic' do - let(:finder) { ComposerMessagesFinder.new(user, composerAction: 'createTopic') } + let(:finder) { ComposerMessagesFinder.new(user, composer_action: 'createTopic') } before do SiteSetting.stubs(:educate_until_posts).returns(10) @@ -42,7 +42,7 @@ describe ComposerMessagesFinder do end context 'creating reply' do - let(:finder) { ComposerMessagesFinder.new(user, composerAction: 'reply') } + let(:finder) { ComposerMessagesFinder.new(user, composer_action: 'reply') } before do SiteSetting.stubs(:educate_until_posts).returns(10) @@ -64,7 +64,7 @@ describe ComposerMessagesFinder do let(:user) { Fabricate.build(:user) } context 'replying' do - let(:finder) { ComposerMessagesFinder.new(user, composerAction: 'reply') } + let(:finder) { ComposerMessagesFinder.new(user, composer_action: 'reply') } it "has no message when `posted_too_much_in_topic?` is false" do user.expects(:posted_too_much_in_topic?).returns(false) @@ -80,7 +80,7 @@ describe ComposerMessagesFinder do end context '.check_avatar_notification' do - let(:finder) { ComposerMessagesFinder.new(user, composerAction: 'createTopic') } + let(:finder) { ComposerMessagesFinder.new(user, composer_action: 'createTopic') } let(:user) { Fabricate(:user) } context "success" do @@ -141,16 +141,16 @@ describe ComposerMessagesFinder do end it "does not give a message for new topics" do - finder = ComposerMessagesFinder.new(user, composerAction: 'createTopic') + finder = ComposerMessagesFinder.new(user, composer_action: 'createTopic') expect(finder.check_sequential_replies).to be_blank end it "does not give a message without a topic id" do - expect(ComposerMessagesFinder.new(user, composerAction: 'reply').check_sequential_replies).to be_blank + expect(ComposerMessagesFinder.new(user, composer_action: 'reply').check_sequential_replies).to be_blank end context "reply" do - let(:finder) { ComposerMessagesFinder.new(user, composerAction: 'reply', topic_id: topic.id) } + let(:finder) { ComposerMessagesFinder.new(user, composer_action: 'reply', topic_id: topic.id) } it "does not give a message to users who are still in the 'education' phase" do user.stubs(:post_count).returns(9) @@ -216,16 +216,16 @@ describe ComposerMessagesFinder do end it "does not give a message for new topics" do - finder = ComposerMessagesFinder.new(user, composerAction: 'createTopic') + finder = ComposerMessagesFinder.new(user, composer_action: 'createTopic') expect(finder.check_dominating_topic).to be_blank end it "does not give a message without a topic id" do - expect(ComposerMessagesFinder.new(user, composerAction: 'reply').check_dominating_topic).to be_blank + expect(ComposerMessagesFinder.new(user, composer_action: 'reply').check_dominating_topic).to be_blank end context "reply" do - let(:finder) { ComposerMessagesFinder.new(user, composerAction: 'reply', topic_id: topic.id) } + let(:finder) { ComposerMessagesFinder.new(user, composer_action: 'reply', topic_id: topic.id) } it "does not give a message to users who are still in the 'education' phase" do user.stubs(:post_count).returns(9) @@ -288,8 +288,8 @@ describe ComposerMessagesFinder do let(:topic) { Fabricate(:topic) } it "does not give a message without a topic id" do - expect(described_class.new(user, composerAction: 'createTopic').check_reviving_old_topic).to be_blank - expect(described_class.new(user, composerAction: 'reply').check_reviving_old_topic).to be_blank + expect(described_class.new(user, composer_action: 'createTopic').check_reviving_old_topic).to be_blank + expect(described_class.new(user, composer_action: 'reply').check_reviving_old_topic).to be_blank end context "a reply" do @@ -300,12 +300,12 @@ describe ComposerMessagesFinder do it "does not notify if last post is recent" do topic = Fabricate(:topic, last_posted_at: 1.hour.ago) - expect(described_class.new(user, composerAction: 'reply', topic_id: topic.id).check_reviving_old_topic).to be_blank + expect(described_class.new(user, composer_action: 'reply', topic_id: topic.id).check_reviving_old_topic).to be_blank end it "notifies if last post is old" do topic = Fabricate(:topic, last_posted_at: 181.days.ago) - expect(described_class.new(user, composerAction: 'reply', topic_id: topic.id).check_reviving_old_topic).not_to be_blank + expect(described_class.new(user, composer_action: 'reply', topic_id: topic.id).check_reviving_old_topic).not_to be_blank end end @@ -316,12 +316,12 @@ describe ComposerMessagesFinder do it "does not notify if last post is new" do topic = Fabricate(:topic, last_posted_at: 1.hour.ago) - expect(described_class.new(user, composerAction: 'reply', topic_id: topic.id).check_reviving_old_topic).to be_blank + expect(described_class.new(user, composer_action: 'reply', topic_id: topic.id).check_reviving_old_topic).to be_blank end it "does not notify if last post is old" do topic = Fabricate(:topic, last_posted_at: 365.days.ago) - expect(described_class.new(user, composerAction: 'reply', topic_id: topic.id).check_reviving_old_topic).to be_blank + expect(described_class.new(user, composer_action: 'reply', topic_id: topic.id).check_reviving_old_topic).to be_blank end end end diff --git a/spec/controllers/composer_messages_controller_spec.rb b/spec/controllers/composer_messages_controller_spec.rb index 90738f77b..39c39dc2e 100644 --- a/spec/controllers/composer_messages_controller_spec.rb +++ b/spec/controllers/composer_messages_controller_spec.rb @@ -10,7 +10,7 @@ describe ComposerMessagesController do context 'when logged in' do let!(:user) { log_in } - let(:args) { {'topic_id' => '123', 'post_id' => '333', 'composerAction' => 'reply'} } + let(:args) { {'topic_id' => '123', 'post_id' => '333', 'composer_action' => 'reply'} } it 'redirects to your user preferences' do xhr :get, :index diff --git a/spec/fabricators/post_fabricator.rb b/spec/fabricators/post_fabricator.rb index 9230eabc8..6b3ff4339 100644 --- a/spec/fabricators/post_fabricator.rb +++ b/spec/fabricators/post_fabricator.rb @@ -110,6 +110,7 @@ Fabricator(:post_with_external_links, from: :post) do raw " Here's a link to twitter: http://twitter.com And a link to google: http://google.com +And a secure link to google: https://google.com And a markdown link: [forumwarz](http://forumwarz.com) And a markdown link with a period after it [codinghorror](http://www.codinghorror.com/blog). " diff --git a/spec/models/topic_link_spec.rb b/spec/models/topic_link_spec.rb index 483b4eebf..7a7109d58 100644 --- a/spec/models/topic_link_spec.rb +++ b/spec/models/topic_link_spec.rb @@ -258,7 +258,7 @@ http://b.com/#{'a'*500} end end - describe 'counts_for and topic_map' do + describe 'query methods' do it 'returns blank without posts' do expect(TopicLink.counts_for(Guardian.new, nil, nil)).to be_blank end @@ -274,6 +274,19 @@ http://b.com/#{'a'*500} TopicLink.counts_for(Guardian.new, post.topic, [post]) end + it 'creates a valid topic lookup' do + TopicLink.extract_from(post) + + lookup = TopicLink.duplicate_lookup(post.topic) + expect(lookup).to be_present + expect(lookup['google.com']).to be_present + + ch = lookup['www.codinghorror.com/blog'] + expect(ch).to be_present + expect(ch[:domain]).to eq('www.codinghorror.com') + expect(ch[:username]).to eq(post.username) + expect(ch[:posted_at]).to be_present + end it 'has the correct results' do TopicLink.extract_from(post) @@ -285,7 +298,7 @@ http://b.com/#{'a'*500} expect(counts_for[post.id].first[:clicks]).to eq(1) array = TopicLink.topic_map(Guardian.new, post.topic_id) - expect(array.length).to eq(4) + expect(array.length).to eq(5) expect(array[0]["clicks"]).to eq("1") end