Skip to content

Commit

Permalink
Regression: users.setStatus throwing an error if message is empty (#1…
Browse files Browse the repository at this point in the history 
…7036)
  • Loading branch information
@MarcosSpessatto
MarcosSpessatto authored Mar 28, 2020
1 parent f925e44 commit af3d888
Show file tree
Hide file tree
Showing 3 changed files with 121 additions and 63 deletions.
4 changes: 3 additions & 1 deletion app/api/server/v1/users.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,7 @@ API.v1.addRoute('users.getStatus', { authRequired: true }, {
if (this.isUserFromParams()) {
const user = Users.findOneById(this.userId);
return API.v1.success({
_id: user._id,
message: user.statusText,
connectionStatus: user.statusConnection,
status: user.status,
Expand All @@ -374,6 +375,7 @@ API.v1.addRoute('users.getStatus', { authRequired: true }, {
const user = this.getUserFromParams();

return API.v1.success({
_id: user._id,
message: user.statusText,
status: user.status,
});
Expand Down Expand Up @@ -403,7 +405,7 @@ API.v1.addRoute('users.setStatus', { authRequired: true }, {
}

Meteor.runAsUser(user._id, () => {
if (this.bodyParams.message || this.bodyParams.message.length === 0) {
if (this.bodyParams.message || this.bodyParams.message === '') {
setStatusText(user._id, this.bodyParams.message);
}
if (this.bodyParams.status) {
Expand Down
10 changes: 10 additions & 0 deletions tests/data/users.helper.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,13 @@ export const getUserByUsername = (username) => new Promise((resolve) => {
resolve(res.body.user);
});
});

export const getUserStatus = (userId) => new Promise((resolve) => {
request.get(api(`users.getStatus?userId=${ userId }`))
.set(credentials)
.expect('Content-Type', 'application/json')
.expect(200)
.end((end, res) => {
resolve(res.body);
});
});
170 changes: 108 additions & 62 deletions tests/end-to-end/api/01-users.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { adminEmail, preferences, password, adminUsername } from '../../data/use
import { imgURL } from '../../data/interactions.js';
import { customFieldText, clearCustomFields, setCustomFields } from '../../data/custom-fields.js';
import { updatePermission, updateSetting } from '../../data/permissions.helper';
import { createUser, login, deleteUser } from '../../data/users.helper.js';
import { createUser, login, deleteUser, getUserStatus } from '../../data/users.helper.js';

describe('[Users]', function() {
this.retries(0);
Expand Down Expand Up @@ -1738,84 +1738,130 @@ describe('[Users]', function() {
});
});

describe('[/users.setStatus]', () => {
let user;
before((done) => {
const username = `user.test.${ Date.now() }`;
const email = `${ username }@rocket.chat`;
request.post(api('users.create'))
describe('[/users.getStatus]', () => {
it('should return my own status', (done) => {
request.get(api('users.getStatus'))
.set(credentials)
.send({ email, name: username, username, password })
.end((err, res) => {
user = res.body.user;
done();
});
});
let userCredentials;
before((done) => {
request.post(api('login'))
.send({
user: user.username,
password,
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
expect(res.body).to.have.property('status');
expect(res.body._id).to.be.equal(credentials['X-User-Id']);
})
.end(done);
});
it('should return other user status', (done) => {
request.get(api('users.getStatus?userId=rocket.cat'))
.set(credentials)
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
userCredentials = {};
userCredentials['X-Auth-Token'] = res.body.data.authToken;
userCredentials['X-User-Id'] = res.body.data.userId;
expect(res.body).to.have.property('success', true);
expect(res.body).to.have.property('status');
expect(res.body._id).to.be.equal('rocket.cat');
})
.end(done);
});
});

describe('[/users.setStatus]', () => {
let user;
before((done) => {
updatePermission('edit-other-user-info', ['admin', 'user']).then(done);
createUser()
.then((createdUser) => {
user = createdUser;
done();
});
});
after((done) => {
request.post(api('users.delete')).set(credentials).send({
userId: user._id,
}).end(() => updatePermission('edit-other-user-info', ['admin']).then(done));
user = undefined;
it('should return an error when the setting "Accounts_AllowUserStatusMessageChange" is disabled', (done) => {
updateSetting('Accounts_AllowUserStatusMessageChange', false).then(() => {
request.post(api('users.setStatus'))
.set(credentials)
.send({
status: 'busy',
message: '',
})
.expect('Content-Type', 'application/json')
.expect(400)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body.errorType).to.be.equal('error-not-allowed');
expect(res.body.error).to.be.equal('Change status is not allowed [error-not-allowed]');
})
.end(done);
});
});
it('should set other user status to busy', (done) => {
const testUsername = `testuserdelete${ +new Date() }`;
let targetUser;
request.post(api('users.register'))
it('should update my own status', (done) => {
updateSetting('Accounts_AllowUserStatusMessageChange', true).then(() => {
request.post(api('users.setStatus'))
.set(credentials)
.send({
status: 'busy',
message: 'test',
})
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
getUserStatus(credentials['X-User-Id']).then((status) => expect(status.status).to.be.equal('busy'));
})
.end(done);
});
});
it('should return an error when trying to update other user status without the required permission', (done) => {
updatePermission('edit-other-user-info', []).then(() => {
request.post(api('users.setStatus'))
.set(credentials)
.send({
status: 'busy',
message: 'test',
userId: user._id,
})
.expect('Content-Type', 'application/json')
.expect(403)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body.error).to.be.equal('unauthorized');
})
.end(done);
});
});
it('should update another user status succesfully', (done) => {
updatePermission('edit-other-user-info', ['admin']).then(() => {
request.post(api('users.setStatus'))
.set(credentials)
.send({
status: 'busy',
message: 'test',
userId: user._id,
})
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
getUserStatus(credentials['X-User-Id']).then((status) => {
expect(status.status).to.be.equal('busy');
expect(status.message).to.be.equal('test');
});
})
.end(done);
});
});
it('should return an error when the user try to update user status with an invalid status', (done) => {
request.post(api('users.setStatus'))
.set(credentials)
.send({
email: `${ testUsername }.@teste.com`,
username: `${ testUsername }test`,
name: testUsername,
pass: password,
status: 'invalid',
})
.expect('Content-Type', 'application/json')
.expect(200)
.expect(400)
.expect((res) => {
targetUser = res.body.user;
expect(res.body).to.have.property('success', false);
expect(res.body.errorType).to.be.equal('error-invalid-status');
expect(res.body.error).to.be.equal('Valid status types include online, away, offline, and busy. [error-invalid-status]');
})
.end(() => {
request.post(api('users.setStatus'))
.set(userCredentials)
.send({
status: 'busy',
message: '',
userId: targetUser._id,
})
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
})
.end(() => {
request.get(api(`users.getStatus?userId=${ targetUser._id }`))
.set(userCredentials)
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('status', 'busy');
})
.end(done);
});
});
.end(done);
});
});

Expand Down

0 comments on commit af3d888

Please sign in to comment.