dombizita commented on code in PR #5658:
URL: https://github.com/apache/ozone/pull/5658#discussion_r1463011454
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/insights/om/om.tsx:
##########
@@ -818,26 +837,30 @@ export class Om extends React.Component<Record<string,
object>, IOmdbInsightsSta
// to fetch previous call stored all prevkey in array and fetching in
respective tabs
if (this.state.activeTab === '2') {
this.setState({
- prevKeyOpen:
openPrevKeyList[openPrevKeyList.indexOf(this.state.prevKeyOpen)-2]
+ prevKeyOpen: prevKeyListMap.get(this.state.pageDisplayCount - 1),
+ pageDisplayCount : this.state.pageDisplayCount - 1
}, () => {
this.fetchOpenKeys(this.state.includeFso, this.state.includeNonFso,
this.state.DEFAULT_LIMIT,this.state.prevKeyOpen);
})
} else if (this.state.activeTab === '3') {
this.setState({
- prevKeyDeletePending:
keysPendingPrevList[keysPendingPrevList.indexOf(this.state.prevKeyDeletePending)-2]
+ prevKeyDeletePending: prevKeyListMap.get(this.state.pageDisplayCount -
1),
+ pageDisplayCount: this.state.pageDisplayCount - 1
}, () => {
this.fetchDeletePendingKeys(this.state.DEFAULT_LIMIT,
this.state.prevKeyDeletePending);
})
} else if (this.state.activeTab === '4') {
this.setState({
- prevKeyDeleted:
deletedKeysPrevList[deletedKeysPrevList.indexOf(this.state.prevKeyDeleted)-2]
+ prevKeyDeleted: prevKeyListMap.get(this.state.pageDisplayCount - 1),
+ pageDisplayCount : this.state.pageDisplayCount- 1
Review Comment:
nit
```suggestion
pageDisplayCount : this.state.pageDisplayCount - 1
```
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/insights/om/om.tsx:
##########
@@ -305,7 +304,8 @@ interface IOmdbInsightsState {
nextClickable: boolean;
includeFso: boolean;
includeNonFso: boolean;
- prevClickable: boolean
+ prevClickable: boolean;
+ pageDisplayCount: number;
Review Comment:
Could you help me understand what this variable is for? Maybe we could add a
comment here, it is used later at many places and for me it's not
straightforward what does it stand for.
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/insights/om/om.tsx:
##########
@@ -387,15 +388,20 @@ export class Om extends React.Component<Record<string,
object>, IOmdbInsightsSta
);
handleExistsAtChange = (e: any) => {
- console.log("handleExistsAtChange", e.key);
- if (e.key === 'OM') {
- mismatchPrevKeyList = [0];
- this.fetchMismatchContainers(this.state.DEFAULT_LIMIT, 0, 'SCM');
- }
- else {
- mismatchPrevKeyList = [0];
- this.fetchMismatchContainers(this.state.DEFAULT_LIMIT, 0, 'OM');
- }
+ this.setState({
+ pageDisplayCount: 1
+ }, () => {
+ if (e.key === 'OM') {
+ //mismatchPrevKeyList = [0];
Review Comment:
This comment should be removed, I think it's not relevant.
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/insights/om/om.tsx:
##########
@@ -387,15 +388,20 @@ export class Om extends React.Component<Record<string,
object>, IOmdbInsightsSta
);
handleExistsAtChange = (e: any) => {
- console.log("handleExistsAtChange", e.key);
- if (e.key === 'OM') {
- mismatchPrevKeyList = [0];
- this.fetchMismatchContainers(this.state.DEFAULT_LIMIT, 0, 'SCM');
- }
- else {
- mismatchPrevKeyList = [0];
- this.fetchMismatchContainers(this.state.DEFAULT_LIMIT, 0, 'OM');
- }
+ this.setState({
+ pageDisplayCount: 1
+ }, () => {
+ if (e.key === 'OM') {
+ //mismatchPrevKeyList = [0];
+ prevKeyListMap.clear();
+ this.fetchMismatchContainers(this.state.DEFAULT_LIMIT, 0, 'SCM');
+ }
+ else {
+ //mismatchPrevKeyList = [0];
Review Comment:
This comment should be removed, I think it's not relevant.
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/insights/om/om.tsx:
##########
@@ -502,13 +512,14 @@ export class Om extends React.Component<Record<string,
object>, IOmdbInsightsSta
})
}
else {
- if (this.state.prevKeyMismatch === 0 ){
+ if (this.state.prevKeyMismatch === 0 || this.state.pageDisplayCount
=== 1){
this.setState({
prevClickable: false
})
}
- if
(mismatchPrevKeyList.includes(mismatchContainersResponse.data.lastKey) ===
false) {
- mismatchPrevKeyList.push(mismatchContainersResponse.data.lastKey);
+ //Need to avoid rewrite in Map so avoiding duplication and wrong value
for
+ if (!prevKeyListMap.has(this.state.pageDisplayCount)) {
Review Comment:
I don't understand this, a map can't have duplicate keys.
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/insights/om/om.tsx:
##########
@@ -856,14 +883,19 @@ export class Om extends React.Component<Record<string,
object>, IOmdbInsightsSta
else {
this.fetchMismatchContainers(this.state.DEFAULT_LIMIT,
this.state.prevKeyMismatch, this.state.mismatchMissingState);
}
+ })
};
itemRender = (_: any, type: string, originalElement: any) => {
if (type === 'prev') {
- return <div>{this.state.prevClickable ? <Link to="/Om"
onClick={this.fetchPreviousRecords}> Prev</Link>: <Link to="/Om" style={{
pointerEvents: 'none' }}>No Records</Link>}</div>;
+ return <>{this.state.prevClickable && this.state.pageDisplayCount &&
<Link to="/Om" className='ant-pagination-item-link'
onClick={this.fetchPreviousRecords}> {'<'}
+ </Link>}</>;
+ }
+ if (type === 'page') {
+ return <>{this.state.pageDisplayCount}</>
}
if (type === 'next') {
- return <div> {this.state.nextClickable ? <Link to="/Om"
onClick={this.fetchNextRecords}> {'>>'} </Link> : <Link to="/Om" style={{
pointerEvents: 'none' }}>No More Further Records</Link>}</div>;
+ return <>{this.state.nextClickable ? <Link to="/Om"
className='ant-pagination-item-link next' onClick={this.fetchNextRecords}>
{'>'} </Link> : <div className='norecords'>No Records</div> }</>;
}
Review Comment:
I think both @devabhishekpal and I suggested
[here](https://github.com/apache/ozone/pull/5658#issuecomment-1881212391) not
to put `No Records` anywhere, nothing should be there if there are no
previous/next pages.
Also this is still inside the `Link` tag, so won't it show as "clickable" if
I hover over it with the cursor?
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/insights/om/om.tsx:
##########
@@ -387,15 +388,20 @@ export class Om extends React.Component<Record<string,
object>, IOmdbInsightsSta
);
handleExistsAtChange = (e: any) => {
- console.log("handleExistsAtChange", e.key);
- if (e.key === 'OM') {
- mismatchPrevKeyList = [0];
- this.fetchMismatchContainers(this.state.DEFAULT_LIMIT, 0, 'SCM');
- }
- else {
- mismatchPrevKeyList = [0];
- this.fetchMismatchContainers(this.state.DEFAULT_LIMIT, 0, 'OM');
- }
+ this.setState({
+ pageDisplayCount: 1
+ }, () => {
+ if (e.key === 'OM') {
+ //mismatchPrevKeyList = [0];
+ prevKeyListMap.clear();
+ this.fetchMismatchContainers(this.state.DEFAULT_LIMIT, 0, 'SCM');
+ }
+ else {
+ //mismatchPrevKeyList = [0];
+ prevKeyListMap.clear();
+ this.fetchMismatchContainers(this.state.DEFAULT_LIMIT, 0, 'OM');
Review Comment:
Same here as above, shouldn't we pass `'SCM'` here?
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/insights/om/om.tsx:
##########
@@ -818,26 +837,30 @@ export class Om extends React.Component<Record<string,
object>, IOmdbInsightsSta
// to fetch previous call stored all prevkey in array and fetching in
respective tabs
if (this.state.activeTab === '2') {
this.setState({
- prevKeyOpen:
openPrevKeyList[openPrevKeyList.indexOf(this.state.prevKeyOpen)-2]
+ prevKeyOpen: prevKeyListMap.get(this.state.pageDisplayCount - 1),
+ pageDisplayCount : this.state.pageDisplayCount - 1
}, () => {
this.fetchOpenKeys(this.state.includeFso, this.state.includeNonFso,
this.state.DEFAULT_LIMIT,this.state.prevKeyOpen);
})
} else if (this.state.activeTab === '3') {
this.setState({
- prevKeyDeletePending:
keysPendingPrevList[keysPendingPrevList.indexOf(this.state.prevKeyDeletePending)-2]
+ prevKeyDeletePending: prevKeyListMap.get(this.state.pageDisplayCount -
1),
+ pageDisplayCount: this.state.pageDisplayCount - 1
}, () => {
this.fetchDeletePendingKeys(this.state.DEFAULT_LIMIT,
this.state.prevKeyDeletePending);
})
} else if (this.state.activeTab === '4') {
this.setState({
- prevKeyDeleted:
deletedKeysPrevList[deletedKeysPrevList.indexOf(this.state.prevKeyDeleted)-2]
+ prevKeyDeleted: prevKeyListMap.get(this.state.pageDisplayCount - 1),
+ pageDisplayCount : this.state.pageDisplayCount- 1
}, () => {
this.fetchDeletedKeys(this.state.DEFAULT_LIMIT,this.state.prevKeyDeleted);
})
}
else {
this.setState({
- prevKeyMismatch:
mismatchPrevKeyList[mismatchPrevKeyList.indexOf(this.state.prevKeyMismatch)-2]
+ prevKeyMismatch: prevKeyListMap.get(this.state.pageDisplayCount -1),
+ pageDisplayCount : this.state.pageDisplayCount- 1
Review Comment:
nit
```suggestion
pageDisplayCount : this.state.pageDisplayCount - 1
```
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/insights/om/om.tsx:
##########
@@ -387,15 +388,20 @@ export class Om extends React.Component<Record<string,
object>, IOmdbInsightsSta
);
handleExistsAtChange = (e: any) => {
- console.log("handleExistsAtChange", e.key);
- if (e.key === 'OM') {
- mismatchPrevKeyList = [0];
- this.fetchMismatchContainers(this.state.DEFAULT_LIMIT, 0, 'SCM');
- }
- else {
- mismatchPrevKeyList = [0];
- this.fetchMismatchContainers(this.state.DEFAULT_LIMIT, 0, 'OM');
- }
+ this.setState({
+ pageDisplayCount: 1
+ }, () => {
+ if (e.key === 'OM') {
+ //mismatchPrevKeyList = [0];
+ prevKeyListMap.clear();
+ this.fetchMismatchContainers(this.state.DEFAULT_LIMIT, 0, 'SCM');
Review Comment:
On line 394 we are checking if the key is equal to `'OM'`, so shouldn't we
fetch with passing `'OM'` here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
