-
Notifications
You must be signed in to change notification settings - Fork 6
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
ERM-999: Add record/row identifiers into Local KB Admin log display #283
Conversation
yarn run v1.22.4 $ eslint lib tests |
1 similar comment
yarn run v1.22.4 $ eslint lib tests |
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.
Looks good to me. Some minor tweaks and linitng errors need to be fixed.
lib/LogsList/LogsList.js
Outdated
render() { | ||
const { job, logs, onNeedMoreLogs, type } = this.props; | ||
resultsFormatter = { | ||
recordDescriptor: ({ additionalInfo }) => { |
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.
recordDescriptor: ({ additionalInfo }) => { | |
recordDescriptor: ({ additionalInfo = {} }) => { | |
const { packageSource, packageSource, rowNumber, recordNumber } = additionalInfo; |
lib/LogsList/LogsList.js
Outdated
<div> | ||
{additionalInfo.packageSource ? <FormattedMessage id="stripes-erm-components.packageSource" tagName="div" values={{ source: additionalInfo.packageSource }} /> : null} | ||
{additionalInfo.packageReference ? <FormattedMessage id="stripes-erm-components.packageReference" tagName="div" values={{ ref: additionalInfo.packageReference }} /> : null} | ||
{additionalInfo.rowNumber ? <FormattedMessage id="stripes-erm-components.rowNumber" tagName="div" values={{ rownumber: additionalInfo.rowNumber }} /> : null} | ||
{additionalInfo.recordNumber ? <FormattedMessage id="stripes-erm-components.recordNumber" tagName="div" values={{ recordnumber: additionalInfo.recordNumber }} /> : null} | ||
</div> |
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.
With the above requested change the additionalInfo.packageSource
could just be replaced with packageSource
. Similarly with packageReference
, rowNumber
and recordNumber
. Its more cleaner that way as we dont need to fetch the property from additionalInfo
everytime.
yarn run v1.22.4 $ eslint lib tests |
yarn run v1.22.4 $ eslint lib tests |
yarn run v1.22.4 $ eslint lib tests |
@aditya-matukumalli Thanks for the suggestions! I updated the code accordingly |
Kudos, SonarCloud Quality Gate passed! 0 Bugs 0.0% Coverage The version of Java (1.8.0_252) you have used to run this analysis is deprecated and we will stop accepting it from October 2020. Please update to at least Java 11. |
No description provided.