[O2B-1056] bookkeeping error page #1820
Conversation
hattinf
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1820 +/- ##
==========================================
+ Coverage 43.78% 43.81% +0.03%
==========================================
Files 893 891 -2
Lines 15954 15887 -67
Branches 3003 2994 -9
==========================================
- Hits 6985 6961 -24
+ Misses 8969 8926 -43 ☔ View full report in Codecov by Sentry. |
|
|
||
| /** | ||
| * Sets the error object for the model | ||
| * @param {Object} error The error object to set, must contain a code, codeDescription and message |
There was a problem hiding this comment.
Please use a precise object description instead of simply suggesting it in comment. Because you use it in different places, I suppose you should create a typedef.
| import { registerFrontLinkListener } from './components/common/navigation/frontLinkListener.js'; | ||
| import { ModalModel } from './components/modal/ModalModel.js'; | ||
| import { BkpRoles } from './domain/enums/BkpRoles.js'; | ||
| import { UserRoleSelectionModel } from './models/UserRoleSelectionModel.js'; | ||
| import { detectorsProvider } from './services/detectors/detectorsProvider.js'; | ||
| import { ObservableData } from './utilities/ObservableData.js'; | ||
| import { debounce, INPUT_DEBOUNCE_TIME } from './utilities/debounce.js'; | ||
| import { getRoleForDetector } from './utilities/getRoleForDetector.js'; | ||
| import { userPreferencesStore } from './utilities/userPreferencesStore.js'; | ||
| import { registerFrontLinkListener } from './components/common/navigation/frontLinkListener.js'; | ||
| import { EosReportModel } from './views/EosReport/EosReportModel.js'; | ||
| import { AboutModel } from './views/About/About.js'; | ||
| import { StatisticsPageModel } from './views/Statistics/StatisticsPageModel.js'; | ||
| import { LhcPeriodsModel } from './views/lhcPeriods/LhcPeriodsModel.js'; | ||
| import { HomePageModel } from './views/Home/Overview/HomePageModel.js'; | ||
| import { DataPassesModel } from './views/DataPasses/DataPassesModel.js'; | ||
| import { SimulationPassesModel } from './views/SimulationPasses/SimulationPassesModel.js'; | ||
| import { EnvironmentModel } from './views/Environments/EnvironmentModel.js'; | ||
| import { EosReportModel } from './views/EosReport/EosReportModel.js'; | ||
| import { ErrorModel } from './views/Error/ErrorModel.js'; | ||
| import Flps from './views/Flps/Flps.js'; | ||
| import { HomePageModel } from './views/Home/Overview/HomePageModel.js'; | ||
| import LhcFills from './views/LhcFills/LhcFills.js'; | ||
| import { LogsModel } from './views/Logs/LogsModel.js'; | ||
| import { QcFlagTypesModel } from './views/QcFlagTypes/QcFlagTypesModel.js'; | ||
| import { QcFlagsModel } from './views/QcFlags/QcFlagsModel.js'; | ||
| import { UserRoleSelectionModel } from './models/UserRoleSelectionModel.js'; | ||
| import { ObservableData } from './utilities/ObservableData.js'; | ||
| import { BkpRoles } from './domain/enums/BkpRoles.js'; | ||
| import { getRoleForDetector } from './utilities/getRoleForDetector.js'; | ||
| import { detectorsProvider } from './services/detectors/detectorsProvider.js'; | ||
| import RunTypeModel from './views/RunTypes/RunTypeModel.js'; | ||
| import { RunsModel } from './views/Runs/RunsModel.js'; | ||
| import { SimulationPassesModel } from './views/SimulationPasses/SimulationPassesModel.js'; | ||
| import { StatisticsPageModel } from './views/Statistics/StatisticsPageModel.js'; | ||
| import Tags from './views/Tags/Tags.js'; |
There was a problem hiding this comment.
You should try to avoid as much as possible this kind of huge changes, for example reordering imports. It adds a lot of noise for review, and someone might do the other way around in a future commit, and this is an endless cycle of changes.
| h('.f5', message), | ||
| h('button.btn.btn-primary', { | ||
| onclick: () => { | ||
| model.router.go('?page=home'); |
There was a problem hiding this comment.
In my opinion, we should avoid as much as possible dependencies inside components. The ideal would be for it to depend only on a very specific submodel, in this case only on model.errorModel. Using router is an external dependency that should be avoided outside of models.
Instead, you should try to have a look at "frontLink" component, that hides this dependency for you (and puts it in one single place). You might style it as a button if you like, but in my personal opinion, a link should look like a link (I am fine with the button if you prefer it).
| model.router.go('?page=home'); | ||
| } }, [ | ||
| iconHome(), | ||
| ' Go to Home Page', |
There was a problem hiding this comment.
You should use flex container and flex-gap instead of putting a space in your text
| await page.setViewport({ | ||
| width: 700, | ||
| height: 720, | ||
| deviceScaleFactor: 1, | ||
| }); |
There was a problem hiding this comment.
Do you need to set this?
| }); | ||
|
|
||
| it('shows the error message', async () => { | ||
| await goToPage(page, 'error'); |
There was a problem hiding this comment.
You can consider that one suite of frontend test is a continuous test, basically you can make them dependent one of the other. In this test for example you can suppose that you are already in the right page, and simply do your tests. Same for the next ones, and this way we avoid navigation.
I have a JIRA ticket
Notable changes for users:
-redirects to error page when page parameter is unknown
Notable changes for developers:
Changes made to the database: