Skip to content

Commit

Permalink
fix: Addressed bug which would cause log detail page to fetch every l…
Browse files Browse the repository at this point in the history
…og entry
  • Loading branch information
MauritsioRK authored and john-ghatas committed May 7, 2020
1 parent f932b2c commit 2a87b58
Show file tree
Hide file tree
Showing 18 changed files with 276 additions and 56 deletions.
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

### Node ###
# Logs
logs
*.log
npm-debug.log*
yarn-debug.log*
Expand Down
39 changes: 39 additions & 0 deletions lib/application/usecases/log/GetLogUseCase.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/**
* @license
* Copyright CERN and copyright holders of ALICE O2. This software is
* distributed under the terms of the GNU General Public License v3 (GPL
* Version 3), copied verbatim in the file "COPYING".
*
* See http://alice-o2.web.cern.ch/license for full licensing information.
*
* In applying this license CERN does not waive the privileges and immunities
* granted to it by virtue of its status as an Intergovernmental Organization
* or submit itself to any jurisdiction.
*/

const { IUseCase } = require('../../interfaces');
const {
repositories: {
LogRepository,
},
utilities: {
TransactionHelper,
},
} = require('../../../database');

/**
* GetLogsUseCase
*/
class GetLogsUseCase extends IUseCase {
/**
* Executes this use case.
*
* @param {Number} id ID of the entity to retrieve.
* @returns {Promise} Promise object represents the result of this use case.
*/
async execute(id) {
return TransactionHelper.provide(() => LogRepository.find(id));
}
}

module.exports = GetLogsUseCase;
2 changes: 2 additions & 0 deletions lib/application/usecases/log/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@

const CreateLogUseCase = require('./CreateLogUseCase');
const GetAllLogsUseCase = require('./GetAllLogsUseCase');
const GetLogUseCase = require('./GetLogUseCase');

module.exports = {
CreateLogUseCase,
GetAllLogsUseCase,
GetLogUseCase,
};
11 changes: 11 additions & 0 deletions lib/database/repositories/LogRepository.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,17 @@ class LogRepository extends ILogRepository {
return Log.findAll().map(LogAdapter.toEntity);
}

/**
* Returns a specific entity.
*
* @param {Number} id ID primary key of the entity to find.
* @returns {Promise|Null} Promise object representing the full mock data
*/
async find(id) {
const result = await Log.findByPk(id);
return result ? [LogAdapter.toEntity(result)] : null;
}

/**
* Insert entity.
*
Expand Down
9 changes: 6 additions & 3 deletions lib/public/Model.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
sessionService,
} from '/js/src/index.js';

import Overview from './views/Overview/Overview.js';
import Logs from './views/Logs/Logs.js';

/**
* Root of model tree
Expand All @@ -38,8 +38,9 @@ export default class Model extends Observable {
this.loader = new Loader(this);
this.loader.bubbleTo(this);

this.overview = new Overview(this);
this.overview.bubbleTo(this);
this.logs = new Logs(this);
this.logs.bubbleTo(this);

// Setup router
this.router = new QueryRouter();
this.router.observe(this.handleLocationChange.bind(this));
Expand All @@ -55,8 +56,10 @@ export default class Model extends Observable {
handleLocationChange() {
switch (this.router.params.page) {
case 'home':
this.logs.fetchAllLogs();
break;
case 'entry':
this.logs.fetchLog(this.router.params.id);
break;
default:
this.router.go('?page=home');
Expand Down
4 changes: 2 additions & 2 deletions lib/public/components/Filters/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ const checkboxFilter = (model, tags) => {
onclick: (e) => {
const isChecked = e.target.checked;
!isChecked
? model.overview.removeFilter(tag)
: model.overview.addFilter(tag);
? model.logs.removeFilter(tag)
: model.logs.addFilter(tag);
},
id: `filtersCheckbox${index + 1}`,
type: 'checkbox',
Expand Down
2 changes: 1 addition & 1 deletion lib/public/components/Table/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ const rowData = (data) => [h('td', data)];
const table = (data, headers, model) => h('table.table.shadow-level1.mh3', [
h('tr', [headers.map((header) => rowHeader(header))]),
data.map((entry, index) => h(`tr#row${index + 1}`, {
style: 'cursor: pointer;',
onclick: () => model.router.go(`?page=entry&id=${entry[0]}`),

}, [Object.keys(entry).map((subItem) => rowData(entry[subItem]))])),
]);

Expand Down
8 changes: 4 additions & 4 deletions lib/public/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@

import { h, switchCase } from '/js/src/index.js';
import NavBar from './components/NavBar/index.js';
import GeneralOverview from './views/Overview/General/page.js';
import DetailsView from './views/Overview/Details/page.js';
import LogsOverview from './views/Logs/Overview/page.js';
import LogDetailView from './views/Logs/Details/page.js';

/**
* Main view layout
Expand All @@ -23,11 +23,11 @@ import DetailsView from './views/Overview/Details/page.js';
*/
export default (model) => {
const navigationPages = {
home: GeneralOverview,
home: LogsOverview,
};

const subPages = {
entry: DetailsView,
entry: LogDetailView,
};

return [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,21 @@ import PostBox from '../../../components/Post/index.js';
* @param {object} model Pass the model to access the defined functions
* @return {vnode} Return the view of the table with the filtering options
*/
const overviewScreen = (model) => {
const data = model.overview.getData();
const id = parseInt(model.router.params.id);
let posts;
const logDetailScreen = (model) => {
const data = model.logs.getData();

data.forEach((entry) => {
if (entry.entryID === id) {
posts = entry.content;
}
});

return h('.w-100.flex-column', [posts.map((post, index) => h('.w-100', PostBox(post, index + 1)))]);
if (data) {
const id = parseInt(model.router.params.id);
const log = data.find((entry) => entry && entry.entryID === id);
return h('.w-100.flex-column', [log.content.map((post, index) => h('.w-100', PostBox(post, index + 1)))]);
} else {
return h('', [
h('.danger', 'This log could not be found.'),
h('button.btn.btn-primary.mv3', {
onclick: () => model.router.go('?page=home'),
}, 'Return to Overview'),
]);
}
};

export default (model) => [overviewScreen(model)];
export default (model) => [logDetailScreen(model)];
Original file line number Diff line number Diff line change
Expand Up @@ -27,37 +27,65 @@ export default class Overview extends Observable {
this.filterCriteria = [];
this.data = [];
this.filtered = [];
this.error = false;
this.headers = ['ID', 'Author ID', 'Title', 'Creation Time'];

this.fetchData();
}

/**
* Get the headers from the Overview class
* @returns {Array} Returns the headers
* Retrieve every relevant log from the API
* @returns {undefined} Injects the data object with the response data
*/
getHeaders() {
return this.headers;
async fetchAllLogs() {
const response = await fetchClient('/api/logs', { method: 'GET' });
const result = await response.json();

if (result.data) {
this.data = result.data;
this.filtered = [...result.data];
this.error = false;
} else {
this.error = true;
}

this.notify();
}

/**
* Fetch all relevant logs data from api
* Retrieve a specified log from the API
* @param {Number} id The ID of the log to be found
* @returns {undefined} Injects the data object with the response data
*/
async fetchData() {
const response = await fetchClient('/api/logs', { method: 'GET' });
const result = await response.json();
this.data = result.data;
this.filtered = [...result.data];
this.notify();
async fetchLog(id) {
// Do not call this endpoint again if we already have all the logs
if (this.data.length < 1) {
const response = await fetchClient(`/api/logs/${id}`, { method: 'GET' });
const result = await response.json();

if (result.data) {
this.data = result.data;
this.error = false;
} else {
this.error = true;
}

this.notify();
}
}

/**
* Get the headers from the Overview class
* @returns {Array} Returns the headers
*/
getHeaders() {
return this.headers;
}

/**
* Getter for all the data
* @returns {Array} Returns all of the data
*/
getData() {
return this.data;
return this.error ? null : this.data;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,15 @@ import { table } from '../../../components/Table/index.js';
* @param {object} model Pass the model to access the defined functions
* @return {vnode} Return the view of the table with the filtering options
*/
const overviewScreen = (model) => {
const headers = model.overview.getHeaders();
const data = model.overview.getDataWithoutTags();
const tags = model.overview.getTagCounts();
const logOverviewScreen = (model) => {
const headers = model.logs.getHeaders();
const data = model.logs.getDataWithoutTags();
const tags = model.logs.getTagCounts();

return h('.w-100.flex-row', [
filters(model, tags),
h('.w-75', [table(data, headers, model)]),
]);
};

export default (model) => [overviewScreen(model)];
export default (model) => [logOverviewScreen(model)];
40 changes: 30 additions & 10 deletions lib/server/controllers/logs.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
* or submit itself to any jurisdiction.
*/

const { log: { CreateLogUseCase, GetAllLogsUseCase } } = require('../../application/usecases');
const { log: { CreateLogUseCase, GetAllLogsUseCase, GetLogUseCase } } = require('../../application/usecases');
const { dtos: { CreateLogDto } } = require('../../domain');

/**
Expand Down Expand Up @@ -145,15 +145,35 @@ const patchRun = (request, response, next) => {
* next middleware function.
* @returns {undefined}
*/
const read = (request, response, next) => {
response.status(501).json({
errors: [
{
status: '501',
title: 'Not implemented',
},
],
});
const read = async (request, response, next) => {
if (isNaN(request.params.id)) {
response.status(400).json({
errors: [
{
status: '400',
title: `Given log id (${request.params.id}) is not a valid number`,
},
],
});
} else {
const log = await new GetLogUseCase()
.execute(request.params.id);

if (log === null) {
response.status(404).json({
errors: [
{
status: '404',
title: `Log with this id (${request.params.id}) could not be found`,
},
],
});
} else {
response.status(200).json({
data: log,
});
}
}
};

/**
Expand Down
15 changes: 15 additions & 0 deletions spec/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,21 @@ paths:
$ref: '#/components/responses/Created'
'400':
$ref: '#/components/responses/BadRequest'
/logs/:id:
get:
parameters:
- name: id
in: path
required: true
schema:
type: number
responses:
'200':
$ref: '#/components/responses/Created'
'400':
$ref: '#/components/responses/BadRequest'
'404':
$ref: '#/components/responses/BadRequest'

components:
schemas:
Expand Down
27 changes: 27 additions & 0 deletions test/application/usecases/log/GetLogUseCase.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/**
* @license
* Copyright CERN and copyright holders of ALICE O2. This software is
* distributed under the terms of the GNU General Public License v3 (GPL
* Version 3), copied verbatim in the file "COPYING".
*
* See http://alice-o2.web.cern.ch/license for full licensing information.
*
* In applying this license CERN does not waive the privileges and immunities
* granted to it by virtue of its status as an Intergovernmental Organization
* or submit itself to any jurisdiction.
*/

const { log: { GetLogUseCase } } = require('../../../../lib/application/usecases');
const chai = require('chai');

const { expect } = chai;

module.exports = () => {
it('should return an array with exactly one object', async () => {
const result = await new GetLogUseCase()
.execute(1);

expect(result).to.be.an('array').that.has.lengthOf(1);
expect(result[0]).to.be.an('object');
});
};
Loading

0 comments on commit 2a87b58

Please sign in to comment.