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

[O2B-1405] Additional query parameters for creating a log #1809

Open
wants to merge 26 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
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
70 changes: 68 additions & 2 deletions lib/public/views/Logs/Create/LogCreationModel.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,14 @@
this._creationTagsPickerModel.bubbleTo(this);
this._creationTagsPickerModel.visualChange$.bubbleTo(this);

const { runNumbers = [], environmentIds = [], lhcFillNumbers = [] } = relations;
const { runNumbers = [], environmentIds = [], lhcFillNumbers = [],
martinboulais marked this conversation as resolved.
Show resolved Hide resolved
templateKey = '', issueDescription = '', detectorOrSubsystem = '' } = relations;

Check warning on line 48 in lib/public/views/Logs/Create/LogCreationModel.js

View check run for this annotation

Codecov / codecov/patch

lib/public/views/Logs/Create/LogCreationModel.js#L47-L48

Added lines #L47 - L48 were not covered by tests
this._runNumbers = runNumbers.join(DATA_DISPLAY_DELIMITER);
this._lhcFills = lhcFillNumbers.join(DATA_DISPLAY_DELIMITER);
this._environments = environmentIds.join(DATA_DISPLAY_DELIMITER);
this._issueDescription = issueDescription;
this._detectorOrSubsystem = detectorOrSubsystem;
this._templateKey = templateKey;

Check warning on line 54 in lib/public/views/Logs/Create/LogCreationModel.js

View check run for this annotation

Codecov / codecov/patch

lib/public/views/Logs/Create/LogCreationModel.js#L52-L54

Added lines #L52 - L54 were not covered by tests
this._isRunNumbersReadonly = Boolean(this._runNumbers);

this._parentLogId = null;
Expand Down Expand Up @@ -93,7 +97,8 @@
this._createdLog = RemoteData.loading();
this.notify();

const { title, text, parentLogId, runNumbers, environments, lhcFills, attachments } = this;
const { title, text, parentLogId, runNumbers, environments, lhcFills,
attachments, templateKey, issueDescription, detectorOrSubsystem } = this;

Check warning on line 101 in lib/public/views/Logs/Create/LogCreationModel.js

View check run for this annotation

Codecov / codecov/patch

lib/public/views/Logs/Create/LogCreationModel.js#L101

Added line #L101 was not covered by tests
const tagsTexts = this._creationTagsPickerModel.selected;
const environmentsIds = environments ? environments.split(',').map((environment) => environment.trim()) : [];
const lhcFillNumbers = lhcFills ? lhcFills.split(',').map((lhcFillNumber) => lhcFillNumber.trim()) : [];
Expand Down Expand Up @@ -125,6 +130,16 @@
options.headers['Content-Type'] = 'application/json';
}

// The field detectorOrSubsystem and issueDescription should be allowed only if the log templateKey is On-call
if (templateKey) {
martinboulais marked this conversation as resolved.
Show resolved Hide resolved
options.body.templateKey = templateKey;

Check warning on line 135 in lib/public/views/Logs/Create/LogCreationModel.js

View check run for this annotation

Codecov / codecov/patch

lib/public/views/Logs/Create/LogCreationModel.js#L134-L135

Added lines #L134 - L135 were not covered by tests

if (templateKey === 'on-call') {
options.body.issueDescription = issueDescription;
options.body.detectorOrSubsystem = detectorOrSubsystem;

Check warning on line 139 in lib/public/views/Logs/Create/LogCreationModel.js

View check run for this annotation

Codecov / codecov/patch

lib/public/views/Logs/Create/LogCreationModel.js#L137-L139

Added lines #L137 - L139 were not covered by tests
martinboulais marked this conversation as resolved.
Show resolved Hide resolved
}
}

try {
const { data: createdLog } = await jsonFetch('/api/logs', options);
this._createdLog = RemoteData.success(createdLog);
Expand Down Expand Up @@ -249,6 +264,57 @@
this.notify();
}

/**
* Returns the system of the log
* @return {string} the system
*/
get detectorOrSubsystem() {
return this._detectorOrSubsystem;

Check warning on line 272 in lib/public/views/Logs/Create/LogCreationModel.js

View check run for this annotation

Codecov / codecov/patch

lib/public/views/Logs/Create/LogCreationModel.js#L271-L272

Added lines #L271 - L272 were not covered by tests
}

/**
* Set the detectorOrSubsystem of the log
* @param {string} value the detectorOrSubsystem
*/
set detectorOrSubsystem(value) {
this._detectorOrSubsystem = value;
this.notify();

Check warning on line 281 in lib/public/views/Logs/Create/LogCreationModel.js

View check run for this annotation

Codecov / codecov/patch

lib/public/views/Logs/Create/LogCreationModel.js#L279-L281

Added lines #L279 - L281 were not covered by tests
}

/**
* Returns the templateKey of the log
* @return {string} the templateKey
*/
get templateKey() {
return this._templateKey;

Check warning on line 289 in lib/public/views/Logs/Create/LogCreationModel.js

View check run for this annotation

Codecov / codecov/patch

lib/public/views/Logs/Create/LogCreationModel.js#L288-L289

Added lines #L288 - L289 were not covered by tests
}

/**
* Set the template of the log
* @param {string} value the templateKey
*/
set templateKey(value) {
this._templateKey = value;
this.notify();

Check warning on line 298 in lib/public/views/Logs/Create/LogCreationModel.js

View check run for this annotation

Codecov / codecov/patch

lib/public/views/Logs/Create/LogCreationModel.js#L296-L298

Added lines #L296 - L298 were not covered by tests
}

/**
* Returns the issueDescription of the log
* @return {string} the issueDescription
*/
get issueDescription() {
return this._issueDescription;

Check warning on line 306 in lib/public/views/Logs/Create/LogCreationModel.js

View check run for this annotation

Codecov / codecov/patch

lib/public/views/Logs/Create/LogCreationModel.js#L305-L306

Added lines #L305 - L306 were not covered by tests
}

/**
* Set the issueDescription of the log
* @param {string} value the issueDescription
*/
set issueDescription(value) {
this._issueDescription = value;
this.notify();

Check warning on line 315 in lib/public/views/Logs/Create/LogCreationModel.js

View check run for this annotation

Codecov / codecov/patch

lib/public/views/Logs/Create/LogCreationModel.js#L313-L315

Added lines #L313 - L315 were not covered by tests
}

/**
* Returns the id of the parent log if it exists
*
Expand Down
13 changes: 10 additions & 3 deletions lib/public/views/Logs/Create/TemplatedLogCreationModel.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,16 @@
* Return a new instance of log template for the given key
*
* @param {logTemplateKey} key the template key
* @param {LogCreationRelations} relations the relations of the log
* @return {LogTemplate|null} the new log template
*/
const logTemplatesFactory = (key) => {
const logTemplatesFactory = (key, relations) => {

Check warning on line 33 in lib/public/views/Logs/Create/TemplatedLogCreationModel.js

View check run for this annotation

Codecov / codecov/patch

lib/public/views/Logs/Create/TemplatedLogCreationModel.js#L33

Added line #L33 was not covered by tests
martinboulais marked this conversation as resolved.
Show resolved Hide resolved
const templateClass = {
['on-call']: OnCallLogTemplate,
['rc-daily-meeting']: RcDailyMeetingTemplate,
}[key] ?? null;
if (templateClass) {
return new templateClass();
return new templateClass(relations);

Check warning on line 39 in lib/public/views/Logs/Create/TemplatedLogCreationModel.js

View check run for this annotation

Codecov / codecov/patch

lib/public/views/Logs/Create/TemplatedLogCreationModel.js#L39

Added line #L39 was not covered by tests
}
return null;
};
Expand All @@ -53,6 +54,8 @@
constructor(onCreation, relations) {
super(onCreation, relations);

this._relations = relations;

Check warning on line 57 in lib/public/views/Logs/Create/TemplatedLogCreationModel.js

View check run for this annotation

Codecov / codecov/patch

lib/public/views/Logs/Create/TemplatedLogCreationModel.js#L57

Added line #L57 was not covered by tests
martinboulais marked this conversation as resolved.
Show resolved Hide resolved

/**
* @type {logTemplateKey|null}
* @private
Expand All @@ -64,6 +67,10 @@
* @private
*/
this._templateModel = null;

if (relations.templateKey) {
this.useTemplate(relations.templateKey);

Check warning on line 72 in lib/public/views/Logs/Create/TemplatedLogCreationModel.js

View check run for this annotation

Codecov / codecov/patch

lib/public/views/Logs/Create/TemplatedLogCreationModel.js#L71-L72

Added lines #L71 - L72 were not covered by tests
}
}

/**
Expand All @@ -73,7 +80,7 @@
* @return {void}
*/
useTemplate(key) {
const templateModel = logTemplatesFactory(key);
const templateModel = logTemplatesFactory(key, this._relations);

Check warning on line 83 in lib/public/views/Logs/Create/TemplatedLogCreationModel.js

View check run for this annotation

Codecov / codecov/patch

lib/public/views/Logs/Create/TemplatedLogCreationModel.js#L83

Added line #L83 was not covered by tests
if (templateModel) {
templateModel.bubbleTo(this);
}
Expand Down
2 changes: 1 addition & 1 deletion lib/public/views/Logs/Create/logCreationComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ export const logCreationComponent = (creationModel) => {
'select.f5.form-control',
{ onchange: (e) => creationModel.useTemplate(e.target.value) },
[
h('option', { value: null, selected: creationModel.templateKey === null }, '- No template -'),
h('option', { value: 'null', selected: creationModel.templateKey === null }, '- No template -'),
martinboulais marked this conversation as resolved.
Show resolved Hide resolved
...Object.entries(templates)
.map(([template, configuration]) => h(
'option',
Expand Down
11 changes: 10 additions & 1 deletion lib/public/views/Logs/Create/templates/OnCallLogTemplate.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,10 @@
export class OnCallLogTemplate extends Observable {
/**
* Constructor
*
* @param {LogCreationRelations} relations the relations of the log
*/
constructor() {
constructor(relations) {

Check warning on line 55 in lib/public/views/Logs/Create/templates/OnCallLogTemplate.js

View check run for this annotation

Codecov / codecov/patch

lib/public/views/Logs/Create/templates/OnCallLogTemplate.js#L55

Added line #L55 was not covered by tests
super();

/**
Expand All @@ -73,6 +75,13 @@
reason: '',
alreadyTakenActions: '',
};

if (relations.detectorOrSubsystem) {
this.formData.detectorOrSubsystem = relations.detectorOrSubsystem;

Check warning on line 80 in lib/public/views/Logs/Create/templates/OnCallLogTemplate.js

View check run for this annotation

Codecov / codecov/patch

lib/public/views/Logs/Create/templates/OnCallLogTemplate.js#L79-L80

Added lines #L79 - L80 were not covered by tests
}
if (relations.issueDescription) {
this.formData.issueDescription = relations.issueDescription;

Check warning on line 83 in lib/public/views/Logs/Create/templates/OnCallLogTemplate.js

View check run for this annotation

Codecov / codecov/patch

lib/public/views/Logs/Create/templates/OnCallLogTemplate.js#L82-L83

Added lines #L82 - L83 were not covered by tests
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
h('option', { disabled: null, selected: true }, '- None -'),
...systems.map((detectorOrSubsystem) => h(
'option',
{ value: detectorOrSubsystem },
{ value: detectorOrSubsystem, selected: creationModel._detectorOrSubsystem === detectorOrSubsystem },
detectorOrSubsystem,
)),
],
Expand Down Expand Up @@ -145,7 +145,7 @@
h(PanelComponent, { class: 'flex-column flex-grow' }, [
h(LabelPanelHeaderComponent, { required: true, for: 'issue-description' }, 'Description of the issue'),
markdownInput(
template.formData.issueDescription,
template.formData.issueDescription || creationModel._issueDescription,

Check warning on line 148 in lib/public/views/Logs/Create/templates/onCallLogCreationConfiguration.js

View check run for this annotation

Codecov / codecov/patch

lib/public/views/Logs/Create/templates/onCallLogCreationConfiguration.js#L148

Added line #L148 was not covered by tests
{
id: 'issue-description',
onchange: (e) => template.patchFormData({ issueDescription: e.target.value }),
Expand Down
11 changes: 10 additions & 1 deletion lib/public/views/Logs/LogsModel.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@
* @param {string} [queryParams.environmentIds] the environment ids to link to the log being created
* @return {void}
*/
loadCreation({ runNumbers, lhcFillNumbers, environmentIds }) {
loadCreation({ runNumbers, lhcFillNumbers, environmentIds, templateKey, issueDescription, detectorOrSubsystem }) {

Check warning on line 116 in lib/public/views/Logs/LogsModel.js

View check run for this annotation

Codecov / codecov/patch

lib/public/views/Logs/LogsModel.js#L116

Added line #L116 was not covered by tests
martinboulais marked this conversation as resolved.
Show resolved Hide resolved
const relations = {};

if (runNumbers) {
Expand All @@ -125,6 +125,15 @@
if (environmentIds) {
relations.environmentIds = environmentIds.split(',').map((environmentId) => environmentId.trim());
}
if (templateKey) {
relations.templateKey = templateKey;

Check warning on line 129 in lib/public/views/Logs/LogsModel.js

View check run for this annotation

Codecov / codecov/patch

lib/public/views/Logs/LogsModel.js#L128-L129

Added lines #L128 - L129 were not covered by tests
}
if (issueDescription) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do you need this here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it ?

relations.issueDescription = issueDescription;

Check warning on line 132 in lib/public/views/Logs/LogsModel.js

View check run for this annotation

Codecov / codecov/patch

lib/public/views/Logs/LogsModel.js#L131-L132

Added lines #L131 - L132 were not covered by tests
}
if (detectorOrSubsystem) {
relations.detectorOrSubsystem = detectorOrSubsystem;

Check warning on line 135 in lib/public/views/Logs/LogsModel.js

View check run for this annotation

Codecov / codecov/patch

lib/public/views/Logs/LogsModel.js#L134-L135

Added lines #L134 - L135 were not covered by tests
}

this._creationModel = new TemplatedLogCreationModel(this.handleLogCreation.bind(this), relations);
this._creationModel.bubbleTo(this);
Expand Down
11 changes: 10 additions & 1 deletion lib/server/services/log/LogService.js
Original file line number Diff line number Diff line change
Expand Up @@ -199,16 +199,25 @@ class LogService {
* @param {string[]} environments the environmentIds to attach to log
* @param {number[]} lhcFills the LHC fill numbers associated with this log
* @param {Attachment[]} attachments the list of attachments to link to the log
* @param {string} templateKey the template key used to create the log
* @param {string} issueDescription the description of the log
* @param {string} detectorOrSubsystem the system of the log
* @return {Promise<Log>} resolve with the created log instance
*/
async create(newLog, runNumbers = [], tagsTexts = [], environments = [], lhcFills = [], attachments = []) {
async create(
newLog, runNumbers = [], tagsTexts = [], environments = [], lhcFills = [],
attachments = [], templateKey = '', issueDescription = '', detectorOrSubsystem = '',
) {
const logId = await createLog(
logAdapter.toDatabase(newLog),
runNumbers,
tagsTexts,
environments,
lhcFills,
attachments.map((attachment) => attachmentAdapter.toDatabase(attachment)),
templateKey,
issueDescription,
detectorOrSubsystem,
martinboulais marked this conversation as resolved.
Show resolved Hide resolved
);
// No transaction, log is ready here and can not be null
return this.get(logId);
Expand Down
46 changes: 46 additions & 0 deletions test/public/logs/create.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,52 @@ module.exports = () => {
await expectInputValue(page, 'input#lhc-fills', '1,2,3');
});

it('Should set the correct template when templateKey is specified without setting other values.', async () => {
const templateKey = 'on-call';
await goToPage(page, `log-create&templateKey=${templateKey}`);

await page.waitForSelector('select');
const selectedOption = await page.evaluate(() => document.querySelector('select').value);
expect(selectedOption).to.equal('on-call');

await expectInputValue(page, 'input#run-numbers', '');
await expectInputValue(page, 'input#environments', '');
await expectInputValue(page, 'input#lhc-fills', '');
expect(await page.evaluate(() => document.querySelector('select#detectorOrSubsystem').value)).to.equal('- None -');
await expectInputValue(page, 'textarea#issue-description', '');
martinboulais marked this conversation as resolved.
Show resolved Hide resolved
});

it('Should autofill detectorOrSubsystem and issueDescription if templateKey is "on-call".', async () => {
const templateKey = 'on-call';
const detectorOrSubsystem = 'ALL';
const issueDescription = 'This is a sample issue description';
await goToPage(
page,
`log-create&templateKey=${templateKey}&detectorOrSubsystem=${detectorOrSubsystem}&` +
`issueDescription=${issueDescription}`,
);

await expectInputValue(page, 'input#run-numbers', '');
await expectInputValue(page, 'input#environments', '');
await expectInputValue(page, 'input#lhc-fills', '');
expect(await page.evaluate(() => document.querySelector('select#detectorOrSubsystem').value)).to.equal('ALL');
await expectInputValue(page, 'textarea#issue-description', issueDescription);
});

it('Should autofill all inputs with provided full parameters.', async () => {
await goToPage(
page,
'log-create&runNumbers=1,2,3&lhcFillNumbers=1,2,3&environmentIds=1,2,3&templateKey=on-call&detectorOrSubsystem=ALL&' +
'issueDescription=This is a sample issue description',
);

await expectInputValue(page, 'input#run-numbers', '1,2,3');
await expectInputValue(page, 'input#environments', '1,2,3');
await expectInputValue(page, 'input#lhc-fills', '1,2,3');
expect(await page.evaluate(() => document.querySelector('select#detectorOrSubsystem').value)).to.equal('ALL');
await expectInputValue(page, 'textarea#issue-description', 'This is a sample issue description');
});

it('should successfully provide a tag picker with search input', async () => {
await waitForNavigation(page, () => pressElement(page, '#create-log-button'));

Expand Down
Loading