-
Notifications
You must be signed in to change notification settings - Fork 1
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
LPA dashboard increment #192
Conversation
…ve into separate file for easier testing
Feat/dashboard
Feat/get real data from datasette for the dashboard
if (!name) { | ||
// throw new Error(`Can't find a name for ${slug}`) | ||
// ToDo: work out what to do here? potentially update it with data from datasette | ||
logger.error(`can't find a name for ${slug}`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call, we shouldn't blow up, the slug is (in theory) readable, so is't ok to proceed. But please lower the level of the statement to warning
(with error
reserved pretty much for "we just need to crash because things got so bad")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You missed one other logger.error(...)
, so just updated it.
Ticket: https://github.com/orgs/digital-land/projects/7/views/9?pane=issue&itemId=72093714
this ticket is to merge staging into main, the diff between staging and main mostly consists of the addition of the lpa dashboard
note that all this code has been previously peer reviewed to get into the staging branch