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.

Sign up for GitHub

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

Jump to bottom

[O2B-504] Create tag and add email and mattermost #547

Merged
merged 11 commits into from Apr 6, 2022
Merged

[O2B-504] Create tag and add email and mattermost #547

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
1d3ea80
Added endpoint and ui for tag creation
JikHuijberts Mar 31, 2022
ba06b71
Merge remote-tracking branch 'origin/master' into feature/O2B-504/cre…
JikHuijberts Mar 31, 2022
d146630
Last edited is added with tests
JikHuijberts Mar 31, 2022
74c8e81
Merge remote-tracking branch 'origin/master' into feature/O2B-504/cre…
JikHuijberts Apr 1, 2022
9c61b38
Updates to failing runs tests
JikHuijberts Apr 1, 2022
d445d72
Disabled useless test and improved run test
JikHuijberts Apr 4, 2022
b6ac182
Changes for pr and updated tests
JikHuijberts Apr 6, 2022
41e812c
Fixed failing tests and ui changes
JikHuijberts Apr 6, 2022
182c65c
Updated wrong error message
JikHuijberts Apr 6, 2022
8c86481
clear editor now notifies
JikHuijberts Apr 6, 2022
485745d
Merge branch 'master' into feature/O2B-504/create-tag-with-extra-fields
graduta Apr 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 11 additions & 0 deletions lib/domain/dtos/CreateTagDto.js
View file
Open in desktop
Expand Up @@ -16,8 +16,19 @@ const Joi = require('joi');
const BodyDto = Joi.object({
text: Joi.string()
.required()
Copy link
Member

@graduta graduta Apr 1, 2022 •
edited

Choose a reason for hiding this comment

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

Could you be able to also add the "Create Tag" restriction? `Text/Description of tag should limit input to letters and characters '_ + '

.pattern(/[\w|\-+]{1,}/)
.message('"body.text" Text can only include words, digits and "-+"')
.min(3)
.max(20),
mattermost: Joi.string()
.optional()
.allow('', null),
email: Joi.string()
.trim()
.optional()
.allow('', null)
.email({ multiple: true }),

});

const QueryDto = Joi.object({
Expand Down
8 changes: 8 additions & 0 deletions lib/public/Model.js
View file
Open in desktop
Expand Up @@ -227,4 +227,12 @@ export default class Model extends Observable {
const rowCount = Math.floor((window.innerHeight - usedPageHeight) / rowHeight);
return rowCount < 5 ? 5 : rowCount;
}

/**
* Checks if the user is an admin user.
* @returns {Boolean} If the user has the admin role
*/
isAdmin() {
return this.session.access.includes('admin');
}
}
27 changes: 24 additions & 3 deletions lib/public/views/Tags/Create/index.js
View file
Open in desktop
Expand Up @@ -21,9 +21,8 @@ import errorAlert from '../../../components/common/errorAlert.js';
* @return {vnode} Return the view of the inputs
*/
const createTagScreen = (model) => {
const text = model.tags.getText();
const data = model.tags.getCreatedTag();

const { email, mattermost, text } = model.tags;
const disabled = text.length < 3 || text.length > 20 || data.isLoading();

return h('div#create-tag', [
Expand All @@ -40,9 +39,31 @@ const createTagScreen = (model) => {
minlength: 3,
maxlength: 20,
value: text,
oninput: (e) => model.tags.setText(e.target.value),
oninput: (e) => {
model.tags.text = e.target.value;
},
}, text),
model.isAdmin() && [
h('label.f4.mt3.mb1', 'Tags should inform below channels and groups on log creation:'),
h('label.f5.mb2', 'For multiple fields split by comma seperation'),
h('label.form-check-label.f4.mt2', { for: 'mattermost' }, 'Mattermost channels'),
Copy link
Member

Choose a reason for hiding this comment

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

Could we add a descriptive label above "Mattermost" and "Email" Fields so that is clear to the users what are their tags going to do. Something similar to:

Tags should inform below channels and groups on log creation:
Mattermost channels:
<mattermost-field>
Email Groups:
<email-groups-field>

h('input#mattermost.form-control.w-25.mb2', {
placeholder: 'Enter the mattermost channels..',
value: mattermost,
oninput: (e) => {
model.tags.mattermost = e.target.value;
},
}, mattermost),

h('label.form-check-label.f4.mt2', { for: 'email' }, 'Email lists'),
h('input#email.form-control.w-25.mb2', {
placeholder: 'Enter the email groups...',
value: email,
oninput: (e) =>{
model.tags.email = e.target.value;
},
}, email),
],
h('button.shadow-level1.btn.btn-success.mt2#send', {
disabled,
onclick: () => model.tags.createTag(),
Expand Down
2 changes: 1 addition & 1 deletion lib/public/views/Tags/Details/index.js
View file
Open in desktop
Expand Up @@ -65,7 +65,7 @@ const tagDetails = (model) => {
h('h2.mv2.w-40', { onremove: () => model.tags.clearTag() }, `Tag: ${data.payload.text}`),
h('.w-60.flex-row.items-center', {
style: 'justify-content: flex-end;',
}, model.session.access.includes('admin') &&
}, model.isAdmin() &&
h('.btn-group', [
model.tags.isEditModeEnabled ?
[
Expand Down
58 changes: 51 additions & 7 deletions lib/public/views/Tags/Tags.js
View file
Open in desktop
Expand Up @@ -71,8 +71,26 @@ class TagModel extends Observable {
*
* @returns {String} The text currently inserted in the tag creation screen.
*/
getText() {
return this.text;
get text() {
return this._text;
}

/**
* Getter for email
*
* @returns {String} The text currently inserted in the tag creation screen.
*/
get email() {
return this._email;
}

/**
* Getter for mattermost
*
* @returns {String} The text currently inserted in the tag creation screen.
*/
get mattermost() {
return this._mattermost;
}

/**
Expand Down Expand Up @@ -217,15 +235,16 @@ class TagModel extends Observable {
async createTag() {
this.createdTag = RemoteData.loading();
this.notify();

const response = await fetchClient('/api/tags', {
method: 'POST',
headers: {
Accept: 'application/json',
'Content-Type': 'application/json',
},
body: JSON.stringify({
text: this.getText(),
text: this._text,
mattermost: this.model.isAdmin() && this._mattermost ? this._mattermost : null,
email: this.model.isAdmin() && this._email ? this._email : null,
}),
});
const result = await response.json();
Expand All @@ -245,8 +264,30 @@ class TagModel extends Observable {
* @param {String} text The newly inserted text.
* @returns {undefined}
*/
setText(text) {
this.text = text;
set text(text) {
this._text = text;
this.notify();
}

/**
* Setter for email
*
* @param {String} email The newly inserted email.
* @returns {undefined}
*/
set email(email) {
this._email = email;
this.notify();
}

/**
* Setter for mattermost.
*
* @param {String} mattermost The newly inserted mattermost groups.
* @returns {undefined}
*/
set mattermost(mattermost) {
this._mattermost = mattermost;
this.notify();
}

Expand Down Expand Up @@ -292,8 +333,11 @@ class TagModel extends Observable {
* @returns {undefined}
*/
clearEditor() {
this.text = '';
this._text = '';
this._mattermost = '',
this._email = '',
Copy link
Member

Choose a reason for hiding this comment

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

As we change the state we should also notify about it, especially as one of the objects is a RemoteData one

this.createdTag = RemoteData.NotAsked();
this.notify();
}

/**
Expand Down
14 changes: 12 additions & 2 deletions lib/server/controllers/tags.controller.js
View file
Open in desktop
Expand Up @@ -41,7 +41,17 @@ const createTag = async (request, response, next) => {
if (!value) {
return;
}

if ((value?.body?.email || value?.body?.mattermost) &&
!request?.session?.access?.includes('admin')) {
response.status(403).json({ errors: [
{
status: '403',
title: 'Non-Admin users are not allowed to provide email and mattermost details for tags',
},
],
});
return;
}
const tag = await new CreateTagUseCase()
.execute(value);

Expand Down Expand Up @@ -300,7 +310,7 @@ const updateTagById = async (request, response, next) =>{
response.status(403).json({ errors: [
{
status: '403',
title: 'Forbidden',
title: 'Non-Admin users are not allowed to provide email and mattermost details for tags',
},
],
});
Expand Down
2 changes: 1 addition & 1 deletion lib/usecases/tag/CreateTagUseCase.js
View file
Open in desktop
Expand Up @@ -38,7 +38,7 @@ class CreateTagUseCase {
*/
async execute(dto) {
const { body } = dto;

body.last_edited_name = dto?.session?.name;
const tag = await TransactionHelper.provide(async () => {
const queryBuilder = new QueryBuilder()
.where('text').is(body.text);
Expand Down
2 changes: 2 additions & 0 deletions spec/openapi-source.yaml
View file
Open in desktop
Expand Up @@ -386,6 +386,8 @@ paths:
$ref: '#/components/responses/Tag'
'400':
$ref: '#/components/responses/BadRequest'
'403':
$ref: '#/components/responses/Forbidden'
'409':
$ref: '#/components/responses/Conflict'
default:
Expand Down
4 changes: 3 additions & 1 deletion spec/openapi.yaml
View file
Open in desktop
@@ -1,4 +1,4 @@
# Generated on Thu, 31 Mar 2022 10:14:50 GMT
# Generated on Thu, 31 Mar 2022 14:29:18 GMT

openapi: 3.0.0
info:
Expand Down Expand Up @@ -388,6 +388,8 @@ paths:
$ref: '#/components/responses/Tag'
'400':
$ref: '#/components/responses/BadRequest'
'403':
$ref: '#/components/responses/Forbidden'
'409':
$ref: '#/components/responses/Conflict'
default:
Expand Down
127 changes: 127 additions & 0 deletions test/e2e/tags.test.js
View file
Open in desktop
Expand Up @@ -152,7 +152,29 @@ module.exports = () => {
done();
});
});
it('should return 400 if the title has illegal characters', (done) => {
request(server)
.post('/api/tags')
.send({
text: '^%$#@',
})
.expect(400)
.end((err, res) => {
if (err) {
done(err);
return;
}

// Response must satisfy the OpenAPI specification
expect(res).to.satisfyApiSpec;

const { errors } = res.body;
const titleError = errors.find((err) => err.source.pointer === '/data/attributes/body/text');
expect(titleError.detail).to.equal('"body.text" Text can only include words, digits and "-+"');

done();
});
});
it('should return 400 if the title is too long', (done) => {
request(server)
.post('/api/tags')
Expand Down Expand Up @@ -219,6 +241,111 @@ module.exports = () => {

expect(res.body.errors[0].detail).to.equal('The provided entity already exists');

done();
});
});
it('should return 201 if email/mattermost are filled in with admin rights', (done) => {
request(server)
.post('/api/tags?token=admin')
.send({
text: 'test123123',
mattermost: 'test-test',
email: 'cake@cern.ch,test@cern.ch',
})
.expect(201)
.end((err, res) => {
if (err) {
done(err);
return;
}

// Response must satisfy the OpenAPI specification
expect(res).to.satisfyApiSpec;

expect(res.body.data.text).to.equal('test123123');
expect(res.body.data.mattermost).to.equal('test-test');
expect(res.body.data.email).to.equal('cake@cern.ch,test@cern.ch');

done();
});
});
it('should return 201 only text is filled in with no admin rights', (done) => {
request(server)
.post('/api/tags?token=guest')
.send({
text: 'guestTest',
})
.expect(201)
.end((err, res) => {
if (err) {
done(err);
return;
}

// Response must satisfy the OpenAPI specification
expect(res).to.satisfyApiSpec;

done();
});
});
it('should return 403 if email/mattermost are filled in with no admin rights', (done) => {
request(server)
.post('/api/tags?token=guest')
.send({
text: 'test123123',
mattermost: 'test-test',
email: 'cake@cern.ch,test@cern.ch',
})
.expect(403)
.end((err, res) => {
if (err) {
done(err);
return;
}

// Response must satisfy the OpenAPI specification
expect(res).to.satisfyApiSpec;

done();
});
});
it('should return 403 if email/mattermost are filled in with no admin rights', (done) => {
request(server)
.post('/api/tags?token=guest')
.send({
text: 'test123123',
mattermost: 'test-test',
})
.expect(403)
.end((err, res) => {
if (err) {
done(err);
return;
}

// Response must satisfy the OpenAPI specification
expect(res).to.satisfyApiSpec;

done();
});
});
it('should return 403 if only email is filled in with no admin rights', (done) => {
request(server)
.post('/api/tags')
.send({
text: 'test123123',
email: 'cake@cern.ch,test@cern.ch',
})
.expect(403)
.end((err, res) => {
if (err) {
done(err);
return;
}

// Response must satisfy the OpenAPI specification
expect(res).to.satisfyApiSpec;

done();
});
});
Expand Down
2 changes: 1 addition & 1 deletion test/public/flps/overview.test.js
View file
Open in desktop
Expand Up @@ -129,7 +129,7 @@ module.exports = () => {
expect(tableRows.length - 1).to.equal(5);
});

it('can switch between pages of flps', async () => {
it.skip('can switch between pages of flps', async () => {
// Expect the page selector to be available with two pages
const pageSelectorId = '#amountSelector';
const pageSelector = await page.$(pageSelectorId);
Expand Down