Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Articles with high IDs can be placed lower than articles with low IDs #1147

Closed
3 tasks done
tucker-m opened this issue Feb 9, 2021 · 0 comments · Fixed by #1148
Closed
3 tasks done

Articles with high IDs can be placed lower than articles with low IDs #1147

tucker-m opened this issue Feb 9, 2021 · 0 comments · Fixed by #1148
Labels
bug

Comments

@tucker-m
Copy link
Contributor

tucker-m commented Feb 9, 2021 •
edited
Loading

  • I have read the CONTRIBUTING.md and followed the provided tips
  • I accept that the issue will be closed without comment if I do not check here
  • I accept that the issue will be closed without comment if I do not fill out all items in the issue template.

Explain the Problem

When set to show new articles at the top, some articles that were downloaded recently show up close to the bottom.

Steps to Reproduce

  1. Add an RSS feed that has 50 items in it, and download all items. So our first feed has articles with ids 1 through 50.
  2. Add a second feed that has 50 items in it. So, the second feed has articles with ids 51 - 100.
  3. Download one more item (id 101) to the first feed. This item won't appear at the top of the feed; it will be near the bottom.

I think I know what's happening. (This is my first time touching Angular, so I could be wrong.) The Nextcloud API returns items like id: "101", with id as a string. Since News is using Angular's default sorting method on the id field, which is an alphabetical comparison, it ranks 101 as being between 1 and 2, rather than after 50.

Parsing the ids as an integer first, and then comparing them, fixes the issue on my instance where I discovered the problem. I still need to work out the test coverage, but I think PR #1148 does the trick.

System Information

  • News app version: 15.1.1
  • Nextcloud version: 21.0.0 alpha
  • Cron type: system cron
  • PHP version: 7.4.3
  • Database and version: Postgres 11
  • Browser and version: Firefox 86
  • OS and version: Ubuntu 20.04.2
@tucker-m tucker-m added the bug label Feb 9, 2021
@tucker-m tucker-m mentioned this issue Feb 9, 2021
Merged
Grotax added a commit that referenced this issue Feb 23, 2021
@Grotax
Release 15.4.0-beta1
c933256 
Changed
- Remove outdated item DB code. ( #1056)
- Stop returning all feeds after marking folder as read. (#1056)
- Always fetch favicon (#1164)
- Use feed logo instead of favicon if it exists and is square (#1164)
- Add CI for item lists (#1180)

Fixed
- Item list throwing error for folder and "all items" (#1180)
- Articles with high IDs can be placed lower than articles with low IDs (#1147)
- Feeds are accidentally moved on rename (#1189)
- Item list not using ID for offset (#1188)

Signed-off-by: Benjamin Brahmer <info@b-brahmer.de>
@Grotax Grotax mentioned this issue Feb 23, 2021
Merged
Grotax added a commit that referenced this issue Feb 23, 2021
@Grotax
Release 15.4.0-beta1
7f0c547 
Changed
- Remove outdated item DB code. ( #1056)
- Stop returning all feeds after marking folder as read. (#1056)
- Always fetch favicon (#1164)
- Use feed logo instead of favicon if it exists and is square (#1164)
- Add CI for item lists (#1180)

Fixed
- Item list throwing error for folder and "all items" (#1180)
- Articles with high IDs can be placed lower than articles with low IDs (#1147)
- Feeds are accidentally moved on rename (#1189)
- Item list not using ID for offset (#1188)

Signed-off-by: Benjamin Brahmer <info@b-brahmer.de>
mnassabain pushed a commit to Team-Forward/news that referenced this issue Mar 1, 2021
@Grotax @mnassabain
Release 15.4.0-beta1
2c88652 
Changed
- Remove outdated item DB code. ( nextcloud#1056)
- Stop returning all feeds after marking folder as read. (nextcloud#1056)
- Always fetch favicon (nextcloud#1164)
- Use feed logo instead of favicon if it exists and is square (nextcloud#1164)
- Add CI for item lists (nextcloud#1180)

Fixed
- Item list throwing error for folder and "all items" (nextcloud#1180)
- Articles with high IDs can be placed lower than articles with low IDs (nextcloud#1147)
- Feeds are accidentally moved on rename (nextcloud#1189)
- Item list not using ID for offset (nextcloud#1188)

Signed-off-by: Benjamin Brahmer <info@b-brahmer.de>
Signed-off-by: Marco Nassabain <marco.nassabain@hotmail.com>
mnassabain pushed a commit to Team-Forward/news that referenced this issue Mar 1, 2021
@Grotax
Release 15.4.0-beta1
4b4a2b3 
Changed
- Remove outdated item DB code. ( nextcloud#1056)
- Stop returning all feeds after marking folder as read. (nextcloud#1056)
- Always fetch favicon (nextcloud#1164)
- Use feed logo instead of favicon if it exists and is square (nextcloud#1164)
- Add CI for item lists (nextcloud#1180)

Fixed
- Item list throwing error for folder and "all items" (nextcloud#1180)
- Articles with high IDs can be placed lower than articles with low IDs (nextcloud#1147)
- Feeds are accidentally moved on rename (nextcloud#1189)
- Item list not using ID for offset (nextcloud#1188)

Signed-off-by: Benjamin Brahmer <info@b-brahmer.de>
Signed-off-by: Marco Nassabain <marco.nassabain@hotmail.com>
mnassabain pushed a commit to Team-Forward/news that referenced this issue Mar 1, 2021
@Grotax @mnassabain
Release 15.4.0-beta1
275770c 
Changed
- Remove outdated item DB code. ( nextcloud#1056)
- Stop returning all feeds after marking folder as read. (nextcloud#1056)
- Always fetch favicon (nextcloud#1164)
- Use feed logo instead of favicon if it exists and is square (nextcloud#1164)
- Add CI for item lists (nextcloud#1180)

Fixed
- Item list throwing error for folder and "all items" (nextcloud#1180)
- Articles with high IDs can be placed lower than articles with low IDs (nextcloud#1147)
- Feeds are accidentally moved on rename (nextcloud#1189)
- Item list not using ID for offset (nextcloud#1188)

Signed-off-by: Benjamin Brahmer <info@b-brahmer.de>
Signed-off-by: Marco Nassabain <marco.nassabain@hotmail.com>
Neo11 pushed a commit to Neo11/news that referenced this issue May 28, 2022
@Grotax
Release 15.4.0-beta1
399249b 
Changed
- Remove outdated item DB code. ( nextcloud#1056)
- Stop returning all feeds after marking folder as read. (nextcloud#1056)
- Always fetch favicon (nextcloud#1164)
- Use feed logo instead of favicon if it exists and is square (nextcloud#1164)
- Add CI for item lists (nextcloud#1180)

Fixed
- Item list throwing error for folder and "all items" (nextcloud#1180)
- Articles with high IDs can be placed lower than articles with low IDs (nextcloud#1147)
- Feeds are accidentally moved on rename (nextcloud#1189)
- Item list not using ID for offset (nextcloud#1188)

Signed-off-by: Benjamin Brahmer <info@b-brahmer.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix id sorting bug tucker-m/news
1 participant
@tucker-m