You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by GitBox <gi...@apache.org> on 2020/06/09 16:16:54 UTC
[GitHub] [couchdb-fauxton] popojargo opened a new pull request #1288: Feature/replication paginate
popojargo opened a new pull request #1288:
URL: https://github.com/apache/couchdb-fauxton/pull/1288
<!-- Thank you for your contribution!
Please file this form by replacing the Markdown comments
with your text. If a section needs no action - remove it.
Also remember, that CouchDB uses the Review-Then-Commit (RTC) model
of code collaboration. Positive feedback is represented +1 from committers
and negative is a -1. The -1 also means veto, and needs to be addressed
to proceed. Once there are no objections, the PR can be merged by a
CouchDB committer.
See: http://couchdb.apache.org/bylaws.html#decisions for more info. -->
## Overview
Add support for pagination in Replicatory activity panel.
<!-- Please give a short brief for the pull request,
what problem it solves or how it makes things better. -->
## Testing recommendations
<!-- Describe how we can test your changes.
Does it provides any behaviour that the end users
could notice? -->
## GitHub issue number
<!-- If this is a significant change, please file a separate issue at:
https://github.com/apache/couchdb-fauxton/issues
and include the number here and in commit message(s) using
syntax like "Fixes #472" or "Fixes apache/couchdb#472". -->
## Related Pull Requests
Supersed #1075
<!-- If your changes affects multiple components in different
repositories please put links to those pull requests here. -->
## Checklist
- [ ] Code is written and works correctly;
- [ ] Changes are covered by tests;
- [ ] Documentation reflects the changes;
- [ ] Update [rebar.config.script](https://github.com/apache/couchdb/blob/master/rebar.config.script) with the correct tag once a new Fauxton release is made
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-fauxton] Antonio-Maranhao commented on pull request #1288: Support pagination in ReplicatorDB activity panel
Posted by GitBox <gi...@apache.org>.
Antonio-Maranhao commented on pull request #1288:
URL: https://github.com/apache/couchdb-fauxton/pull/1288#issuecomment-817986743
@popojargo I'd go with a note above the table. The tooltips are probably too repetitive as you said and less obvious.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-fauxton] popojargo commented on pull request #1288: Support pagination in ReplicatorDB activity panel
Posted by GitBox <gi...@apache.org>.
popojargo commented on pull request #1288:
URL: https://github.com/apache/couchdb-fauxton/pull/1288#issuecomment-817394801
> Besides my suggestion, just wanted to point out that column sorting and filtering only affect the docs in the current page, which I'm sure it'll mislead users.
>
> Maybe it'd be worth adding a note at the top @popojargo @brianewilkins ?
Brainstorming here:
- We could maybe add a tooltip to the sort buttons? That said, that would be a bit repetitive.<
- Another variant of the tooltip could be to add a tooltip to the right of the action bar. We would only have one tooltip for all sort buttons.
- We could add a note above the table. That would take some height some something that might not be important?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-fauxton] Antonio-Maranhao commented on a change in pull request #1288: Support pagination in ReplicatorDB activity panel
Posted by GitBox <gi...@apache.org>.
Antonio-Maranhao commented on a change in pull request #1288:
URL: https://github.com/apache/couchdb-fauxton/pull/1288#discussion_r603632649
##########
File path: app/addons/replication/reducers.js
##########
@@ -406,4 +463,10 @@ export const getReplicateInfo = (state) => {
});
};
+export const getPagination = (state) => state.pagination;
+export const getPageStart = (state) => 1 + (state.pagination.page - 1) * state.pagination.docsPerPage;
+export const getPageEnd = (state) => state.pagination.docsPerPage + (state.pagination.page - 1) * state.pagination.docsPerPage;
Review comment:
Can you use this so it shows the correct range on the last page?
```suggestion
export const getPageEnd = (state) => {
if (state.isLoading || !state.statusDocs || state.statusDocs.length === 0) {
return false;
}
const pageStart = (state.pagination.page - 1) * state.pagination.docsPerPage;
return pageStart + state.statusDocs.length;
};
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-fauxton] popojargo commented on pull request #1288: Support pagination in ReplicatorDB activity panel
Posted by GitBox <gi...@apache.org>.
popojargo commented on pull request #1288:
URL: https://github.com/apache/couchdb-fauxton/pull/1288#issuecomment-641018270
@garrensmith @Antonio-Maranhao Do I have to make some magic for partitioned database?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-fauxton] Antonio-Maranhao commented on a change in pull request #1288: Support pagination in ReplicatorDB activity panel
Posted by GitBox <gi...@apache.org>.
Antonio-Maranhao commented on a change in pull request #1288:
URL: https://github.com/apache/couchdb-fauxton/pull/1288#discussion_r438947056
##########
File path: app/addons/replication/reducers.js
##########
@@ -348,6 +371,38 @@ const replication = (state = initialState, {type, options}) => {
allReplicateSelected: false
};
+ case ActionTypes.REPLICATION_UPDATE_PER_PAGE_RESULTS:
+ const newResultsState = { ...state };
+ newResultsState.pagination.docsPerPage = options;
+ saveDocsPerPage(options);
+ return {
+ ...state,
+ pagination: {
+ ...state.pagination,
+ docsPerPage: options,
+ page: 1,
+ canShowNext: false
+ }
+ };
+
+ case ActionTypes.REPLICATION_NEXT_PAGE:
+ return {
+ ...state,
+ pagination: {
Review comment:
Shouldn't you add `...state.pagination` to preserve the other values?
##########
File path: app/addons/replication/components/common-table.js
##########
@@ -381,7 +381,7 @@ export class ReplicationTable extends React.Component {
render () {
return (
- <Table striped>
+ <Table striped style={{marginBottom: '100px'}}>
<thead>
<tr>
<th className="replication__table-bulk-select">
Review comment:
Unrelated to your changes... but I noticed on lines 403 and 405, it should be `startTime` and not `statusTime` so the sorting matches the values displayed for that column.
##########
File path: app/addons/replication/reducers.js
##########
@@ -348,6 +371,38 @@ const replication = (state = initialState, {type, options}) => {
allReplicateSelected: false
};
+ case ActionTypes.REPLICATION_UPDATE_PER_PAGE_RESULTS:
+ const newResultsState = { ...state };
+ newResultsState.pagination.docsPerPage = options;
+ saveDocsPerPage(options);
+ return {
+ ...state,
+ pagination: {
+ ...state.pagination,
+ docsPerPage: options,
+ page: 1,
+ canShowNext: false
+ }
+ };
+
+ case ActionTypes.REPLICATION_NEXT_PAGE:
+ return {
+ ...state,
+ pagination: {
+ docsPerPage: state.pagination.docsPerPage,
+ page: state.pagination.page + 1
+ }
+ };
+
+ case ActionTypes.REPLICATION_PREVIOUS_PAGE:
+ return {
+ ...state,
+ pagination: {
Review comment:
Same here re: `...state.pagination`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-fauxton] Antonio-Maranhao commented on pull request #1288: Support pagination in ReplicatorDB activity panel
Posted by GitBox <gi...@apache.org>.
Antonio-Maranhao commented on pull request #1288:
URL: https://github.com/apache/couchdb-fauxton/pull/1288#issuecomment-809475570
@popojargo sorry for the delay. I'll make time to review this today or tomorrow.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-fauxton] brianewilkins commented on pull request #1288: Support pagination in ReplicatorDB activity panel
Posted by GitBox <gi...@apache.org>.
brianewilkins commented on pull request #1288:
URL: https://github.com/apache/couchdb-fauxton/pull/1288#issuecomment-802360041
In inspected the commit https://github.com/apache/couchdb-fauxton/pull/1288/commits/a35a657a75d2e046b6699da09be0044be3170e68
and, insofar as I understand it, it all looks good to me.
I do see that the commit contains fixes for the specific faults pointed out by @Antonio-Maranhao at https://github.com/apache/couchdb-fauxton/pull/1288#pullrequestreview-429124811
I have played with it a bit (I created 110 continuous replications to do so) and did not find anything wrong with it.
So +1 from me, if that counts for anything.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org