Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add conversation-part-closed component #1638

Merged
merged 18 commits into from
Jan 16, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions app/components/conversations/conversation-part-closed.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import Component from '@ember/component';
import { inject as service } from '@ember/service';
import { computed, get } from '@ember/object';
import { alias } from '@ember/object/computed';

export default Component.extend({
classNames: ['conversation-part', 'conversation-part--closed'],
classNameBindings: ['isSelf:conversation-part--is-self'],

currentUser: service(),

author: null,
closedAt: null,

user: alias('currentUser.user'),

isSelf: computed('author', 'user', function() {
return get(this, 'author.id') === get(this, 'user.id');
})
});
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { alias } from '@ember/object/computed';
import { inject as service } from '@ember/service';

export default Component.extend({
classNames: ['conversation-part'],
classNames: ['conversation-part', 'conversation-part--comment'],
classNameBindings: ['isSelf:conversation-part--is-self'],

author: null,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<div class="coversation-part__message-wrapper">
<div class="conversation-part__timestamps-wrapper">
<div class="conversation-part__timestamps">
<span data-test-closed-at>
{{#if isSelf}}
You closed this {{moment-from-now closedAt interval=60000}}
{{else}}
{{author.username}} closed this {{moment-from-now closedAt interval=60000}}
{{/if}}
</span>
</div>
</div>
</div>
21 changes: 14 additions & 7 deletions app/templates/components/conversations/conversation-thread.hbs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<div class="conversation-thread-inner">
<div class="conversation-thread__messages">
{{#if conversation.message.isLoaded}}
{{conversations/conversation-part
{{conversations/conversation-part-comment
author=conversation.message.author
body=conversation.message.body
readAt=conversation.message.readAt
Expand All @@ -10,12 +10,19 @@
{{/if}}
{{#each sortedConversationParts as |conversationPart|}}
{{#if conversationPart.isLoaded}}
{{conversations/conversation-part
author=conversationPart.author
body=conversationPart.body
readAt=conversationPart.readAt
sentAt=conversationPart.insertedAt
}}
{{#if (eq conversationPart.partType 'isClosed')}}
{{conversation/conversation-part-closed
author=conversationPart.author
closedAt=conversationPart.insertedAt
}}
{{else}}
{{conversations/conversation-part-comment
author=conversationPart.author
body=conversationPart.body
readAt=conversationPart.readAt
sentAt=conversationPart.insertedAt
}}
{{/if}}
{{/if}}
{{/each}}
</div>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import { moduleForComponent, test } from 'ember-qunit';
import hbs from 'htmlbars-inline-precompile';
import { set } from '@ember/object';
import PageObject from 'ember-cli-page-object';
import component from 'code-corps-ember/tests/pages/components/conversations/conversation-part';
import stubService from 'code-corps-ember/tests/helpers/stub-service';
import moment from 'moment';

let page = PageObject.create(component);

function renderPage() {
page.render(hbs`
{{conversations/conversation-part-closed
author=author
closedAt=closedAt
}}
`);
}

moduleForComponent('conversations/conversation-part-closed', 'Integration | Component | conversations/conversation part closed', {
integration: true,
beforeEach() {
page.setContext(this);
},
afterEach() {
page.removeContext();
}
});

test('if current user closes message, "You closed this" is rendered', function(assert) {
assert.expect(1);

let user = {
id: 1,
username: 'testuser'
};
set(this, 'author', user);
stubService(this, 'current-user', { user });

let twoMinutesAgo = moment().subtract(2, 'minutes');
let twoMinutesAgoFriendly = twoMinutesAgo.from();
set(this, 'closedAt', twoMinutesAgo);

renderPage();

let expectedText = `You closed this ${twoMinutesAgoFriendly}`;
assert.equal(page.closedAt.text, expectedText, 'The closed at timestamp is rendered');
});

test('if someone other than the current user closes the message, "Author.username closed this at" is rendered', function(assert) {
assert.expect(1);

let user = {
id: 1,
username: 'currentuser'
};
stubService(this, 'current-user', { user });

let author = {
id: 2,
username: 'authoruser'
};
set(this, 'author', author);

let twoMinutesAgo = moment().subtract(2, 'minutes');
let twoMinutesAgoFriendly = twoMinutesAgo.from();
set(this, 'closedAt', twoMinutesAgo);

renderPage();

let expectedText = `${author.username} closed this ${twoMinutesAgoFriendly}`;
assert.equal(page.closedAt.text, expectedText, 'The closed at timestamp is rendered');
});
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,15 @@ let page = PageObject.create(component);

function renderPage() {
page.render(hbs`
{{conversations/conversation-part
{{conversations/conversation-part-comment
author=author
body=body
sentAt=sentAt
}}
`);
}

moduleForComponent('conversations/conversation-part', 'Integration | Component | conversations/conversation part', {
moduleForComponent('conversations/conversation-part-comment', 'Integration | Component | conversations/conversation part comment', {
integration: true,
beforeEach() {
page.setContext(this);
Expand Down Expand Up @@ -53,7 +53,7 @@ test('it renders all the details', function(assert) {

assert.equal(page.body.text, body, 'The body renders in the chat bubble');
assert.equal(page.sentAt.text, '2 days ago', 'The sent at timestamp renders');
assert.notOk(page.sentByCurrentUser, 'Does not have the current user styles');
assert.notOk(page.isByCurrentUser, 'Does not have the current user styles');
assert.equal(page.photo.url, user.photoThumbUrl, 'The user photo renders');
assertTooltipNotRendered(assert);

Expand Down Expand Up @@ -82,7 +82,7 @@ test('when the current user did not send the message', function(assert) {

renderPage();

assert.notOk(page.sentByCurrentUser, 'Does not have the current user styles');
assert.notOk(page.isByCurrentUser, 'Does not have the current user styles');
});

test('when the current user sent the message', function(assert) {
Expand All @@ -93,5 +93,5 @@ test('when the current user sent the message', function(assert) {

renderPage();

assert.ok(page.sentByCurrentUser, 'Has the current user styles');
assert.ok(page.isByCurrentUser, 'Has the current user styles');
});
12 changes: 9 additions & 3 deletions tests/pages/components/conversations/conversation-part.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@ export default {
scope: '[data-test-body]'
},

closedAt: {
scope: '[data-test-closed-at]'
},

isComment: hasClass('conversation-part--comment'),
isClosed: hasClass('conversation-part--closed'),
isByCurrentUser: hasClass('conversation-part--is-self'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw a question that got marked as outdated, which caused me to think page objects might not be completely clear to you. They are only ever used in tests and they are defined in a way that provides us with information we need to answer questions in tests.

If a conversation thread lists multiple conversations of different partType values, we want to ask

  • is this part displayed as a comment
  • is this other part displayed as "user closed at"

We know that a comment has a conversation-part--coment class, and we know that the other one has a conversation-part--closed class, so we could ask

assert.ok($('conversation-part').classNames().indexOf('conversation-part--comment') > -1);
assert.ok($('conversation-part').classNames().indexOf('conversation-part--closed') > -1);

and that works, except it's not always completely clear what it is we're actually asking. Also, in multiple tests, we tend to ask the same or similar questions, so we have to repeat all of that. Then, if, for example, the class name changes, we have to change it in all of those places.

What page objects allow us to do is to specify

isClosed: hasClass('conversation-part--closed'),

in just one place and use that.

assert.ok(conversationPart.isClosed);
assert.ok(conversationPart.isComment);

That makes the question much clearer. Also, if the class changes, we change it in just that one place - in the page object.


photo: {
scope: '[data-test-target-photo]',
url: attribute('src')
Expand All @@ -23,7 +31,5 @@ export default {

sentAt: {
scope: '[data-test-sent-at]'
},

sentByCurrentUser: hasClass('conversation-part--is-self')
}
};