Refactor issues #380

Merged
mmarif merged 10 commits from refactor-issues into master 2020-04-14 20:55:09 +00:00
Owner

Will close #309, #373 and #173

To do:

  • check for Gitea version to pass the correct param limit.
Will close #309, #373 and #173 To do: - [x] check for Gitea version to pass the correct param limit.
mmarif added this to the 2.5.0 milestone 2020-04-07 18:36:56 +00:00
mmarif added the
Refactor
UI/UX
WIP
Improvement
labels 2020-04-07 18:36:56 +00:00
mmarif self-assigned this 2020-04-07 18:36:56 +00:00
6543 reviewed 2020-04-07 23:29:14 +00:00
Dismissed
@ -0,0 +9,4 @@
// issues variables
String TAGIssuesList = "IssuesListFragment - ";
int issuesPageInit = 1;
int resultLimitNewGiteaInstances = 50; // Gitea 1.12 and above

I think it should be the other way around. since gitea < 1.12.0 sometimes accept limits but has no pagination.
so if we limit the result to 10 it could happen that we can no longer access the 11th issue and so on.

suggestion: since we have to pass a limit falue to the API: set 100 or so for old installations. an old gitea will ignore it or deliver us at max 100 results. For Gitea v1.12.0 paggination works just fine so we can limit each api call to a smal number of items

I think it should be the other way around. since gitea < 1.12.0 sometimes accept limits but has no pagination. so if we limit the result to 10 it could happen that we can no longer access the 11th issue and so on. suggestion: since we have to pass a limit falue to the API: set 100 or so for old installations. an old gitea will ignore it or deliver us at max 100 results. For Gitea v1.12.0 paggination works just fine so we can limit each api call to a smal number of items
Author
Owner

Adding different limits has many reasons here. 1st of API does not return something like has_more or next_page etc. So these variables are used inside the app to decide what to do next on scroll etc.

These are not strictly for sending to the API, but the recyclerview to know if there is anything to load. Below 1.12 as you mentioned does not take limit to consideration and always returns 10 no matter what. So passing that param as limit is useless(I know that). But it is used inside the load next calls to properly know the exact length of data returned.

As for 1.12 and above there is a limit param now, which is useful if in future we add a setting slider to load how many entries per screen(10 to 50).
But for old versions it will be always 10, and no user interaction is needed.

In 2.4.x there is a bug for pagination when we introduced limit 50, but I ignored it because I was looking forward to fix all these in 2.5.

Hope this does make sense. :)

Adding different limits has many reasons here. 1st of API does not return something like `has_more` or `next_page` etc. So these variables are used inside the app to decide what to do next on scroll etc. These are not strictly for sending to the API, but the recyclerview to know if there is anything to load. Below 1.12 as you mentioned does not take limit to consideration and **always returns 10 no matter what**. So passing that param as limit is useless(I know that). But it is used inside the load next calls to properly know the exact length of data returned. As for 1.12 and above there is a limit param now, which is useful if in future we add a setting slider to load how many entries per screen(10 to 50). But for old versions it will be always 10, and no user interaction is needed. In 2.4.x there is a bug for pagination when we introduced limit 50, but I ignored it because I was looking forward to fix all these in 2.5. Hope this does make sense. :)
mmarif added
LGTM-need
and removed
WIP
labels 2020-04-08 10:58:41 +00:00
Member

Note: there is missmatch with translation - I'll look into this later

Note: there is missmatch with translation - I'll look into this later
Author
Owner

Note: there is missmatch with translation - I’ll look into this later

Ok

> Note: there is missmatch with translation - I’ll look into this later Ok
mmarif added
LGTM-done
and removed
LGTM-need
labels 2020-04-14 20:54:28 +00:00
mmarif merged commit 7caecadb32 into master 2020-04-14 20:55:09 +00:00
mmarif deleted branch refactor-issues 2020-04-14 20:55:23 +00:00
6543 approved these changes 2020-04-14 20:55:53 +00:00
Dismissed
This repo is archived. You cannot comment on pull requests.
No reviewers
No Milestone
No Assignees
2 Participants
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: gitnex/GitNex#380
No description provided.