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.

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

db: no longer order by items.last_modified #1339

Merged

Conversation

SMillerDev
Copy link
Contributor

This might fix some issues some people/apps have been having.

Most notably: #1324

@SMillerDev SMillerDev changed the title db: no longer order by items.last_modified db: no longer order by items.last_modified May 4, 2021
@SMillerDev SMillerDev force-pushed the fix/db/remove_last_modified_ordering branch from e817306 to 8ab38b0 Compare May 4, 2021 19:23
@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (stable15@ea074d0). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head a311847 differs from pull request most recent head 8ab38b0. Consider uploading reports for the commit 8ab38b0 to get more accurate results
Impacted file tree graph

@@             Coverage Diff             @@
##             stable15    #1339   +/-   ##
===========================================
  Coverage            ?   90.03%           
  Complexity          ?      727           
===========================================
  Files               ?       64           
  Lines               ?     2640           
  Branches            ?        0           
===========================================
  Hits                ?     2377           
  Misses              ?      263           
  Partials            ?        0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ea074d0...8ab38b0. Read the comment docs.

@Grotax
Copy link
Member

Grotax commented May 21, 2021

needs a rebase

@mjanssens can you help test this?

@Grotax Grotax linked an issue May 24, 2021 that may be closed by this pull request
3 tasks
@Grotax Grotax added this to the 15.4.5 milestone May 24, 2021
@Grotax Grotax added API Impact API/Backend code 3. to review labels May 24, 2021
@SMillerDev
Copy link
Contributor Author

Seems to be needed

@Grotax
Copy link
Member

Grotax commented May 25, 2021

Ok can you do a rebase, then we can merge it an make a new release

Signed-off-by: Sean Molenaar <sean@seanmolenaar.eu>
@SMillerDev SMillerDev force-pushed the fix/db/remove_last_modified_ordering branch from 8ab38b0 to e4a2f4b Compare May 25, 2021 07:26
@SMillerDev
Copy link
Contributor Author

Done

@mjanssens
Copy link

mjanssens commented May 25, 2021

@mjanssens can you help test this?

I would be happy to help out with testing. Just tell me what I can do.

@Grotax
Copy link
Member

Grotax commented May 25, 2021

You can cd into your nextcloud directory and then
cd apps/news/
and apply the patch as your webserver service user for example debian & co www-data

sudo -u www-data wget https://patch-diff.githubusercontent.com/raw/nextcloud/news/pull/1339.patch
sudo -u www-data git apply 1339.patch
or
sudo -u www-data patch < 1339.patch

@mjanssens
Copy link

mjanssens commented May 25, 2021

Thanks, I did a git apply but got:

error: tests/Unit/Db/ItemMapperAfterTest.php: No such file or directory
error: tests/Unit/Db/ItemMapperTest.php: No such file or directory

Considering these are test files I ran:
sudo -u www-data git apply 1339.patch --exclude=tests/Unit/Db/*

The patch applied, but doesn't fix this issue.
However #1344 appears to be fixed by this, so nice progress 😃

edit:
To be complete: tested on Nextcloud 21.0.2, News app 15.4.4.

@Grotax Grotax merged commit 5ba75f4 into nextcloud:stable15 May 26, 2021
Grotax added a commit that referenced this pull request May 26, 2021
Fixed
- newestId does not return newest ID but last updated (#1339)

Signed-off-by: Benjamin Brahmer <info@b-brahmer.de>
@Grotax Grotax mentioned this pull request May 26, 2021
@Grotax Grotax modified the milestones: 15.4.5, 16.0.0 May 26, 2021
@Grotax
Copy link
Member

Grotax commented May 26, 2021

we will need this also in 16.0.0, I guess

Grotax added a commit that referenced this pull request May 27, 2021
Fixed
- newestId does not return newest ID but last updated (#1339)

Signed-off-by: Benjamin Brahmer <info@b-brahmer.de>
Grotax added a commit that referenced this pull request Jun 1, 2021
Changed
- Allow installation on Nextcloud v22
- Remove deprecated API endpoints and occ comand (#935)
  - /api/v1-2/user
  - /api/v1-2/user/avatar
  - ./occ news:updater:all-feeds
Fixed
- allow calling `/items?getRead=false` without a feed/folder (#1380 #1356)
- newestId does not return newest ID but last updated (#1339)

Signed-off-by: Benjamin Brahmer <info@b-brahmer.de>
@Grotax Grotax mentioned this pull request Jun 1, 2021
Grotax added a commit that referenced this pull request Jun 2, 2021
Changed
- Allow installation on Nextcloud v22
- Remove deprecated API endpoints and occ comand (#935)
  - /api/v1-2/user
  - /api/v1-2/user/avatar
  - ./occ news:updater:all-feeds
Fixed
- allow calling `/items?getRead=false` without a feed/folder (#1380 #1356)
- newestId does not return newest ID but last updated (#1339)

Signed-off-by: Benjamin Brahmer <info@b-brahmer.de>
Grotax added a commit that referenced this pull request Jul 1, 2021
There are no additional changes compared to the latest beta.

Changed
- News now requires a 64bit OS
- v2 API implementation (folder part)
- Implemented sharing news items between nextcloud users (#1191)
- Updated the news items table in DB to include sharer data (#1191)
- Added route for sharing news items (#1191)
- Added share data in news items serialization (#1191)
- Added tests for the news items share feature (#1191)
- Added sharing articles with nextcloud users (#1217)
- Added sharing articles on social media (Facebook, Twitter) or mail (#1217)
- Allow installation on Nextcloud v22
- Remove deprecated API endpoints and occ command (#935)
  - /api/v1-2/user
  - /api/v1-2/user/avatar
  - ./occ news:updater:all-feeds
- added feed search (#1402)

Fixed
- allow calling `/items?getRead=false` without a feed/folder (#1380 #1356)
- newestId does not return newest ID but last updated (#1339)
- removed reference for deleted repair-steps (#1399)
- Fix NotNullConstraintViolation when sharing news items with users (#1406)

Signed-off-by: Benjamin Brahmer <info@b-brahmer.de>
@Grotax Grotax mentioned this pull request Jul 1, 2021
Grotax added a commit that referenced this pull request Jul 4, 2021
There are no additional changes compared to the latest beta.

Changed
- News now requires a 64bit OS
- v2 API implementation (folder part)
- Implemented sharing news items between nextcloud users (#1191)
- Updated the news items table in DB to include sharer data (#1191)
- Added route for sharing news items (#1191)
- Added share data in news items serialization (#1191)
- Added tests for the news items share feature (#1191)
- Added sharing articles with nextcloud users (#1217)
- Added sharing articles on social media (Facebook, Twitter) or mail (#1217)
- Allow installation on Nextcloud v22
- Remove deprecated API endpoints and occ command (#935)
  - /api/v1-2/user
  - /api/v1-2/user/avatar
  - ./occ news:updater:all-feeds
- added feed search (#1402)

Fixed
- allow calling `/items?getRead=false` without a feed/folder (#1380 #1356)
- newestId does not return newest ID but last updated (#1339)
- removed reference for deleted repair-steps (#1399)
- Fix NotNullConstraintViolation when sharing news items with users (#1406)

Signed-off-by: Benjamin Brahmer <info@b-brahmer.de>
Neo11 pushed a commit to Neo11/news that referenced this pull request May 28, 2022
Changed
- Allow installation on Nextcloud v22
- Remove deprecated API endpoints and occ comand (nextcloud#935)
  - /api/v1-2/user
  - /api/v1-2/user/avatar
  - ./occ news:updater:all-feeds
Fixed
- allow calling `/items?getRead=false` without a feed/folder (nextcloud#1380 nextcloud#1356)
- newestId does not return newest ID but last updated (nextcloud#1339)

Signed-off-by: Benjamin Brahmer <info@b-brahmer.de>
Neo11 pushed a commit to Neo11/news that referenced this pull request May 28, 2022
There are no additional changes compared to the latest beta.

Changed
- News now requires a 64bit OS
- v2 API implementation (folder part)
- Implemented sharing news items between nextcloud users (nextcloud#1191)
- Updated the news items table in DB to include sharer data (nextcloud#1191)
- Added route for sharing news items (nextcloud#1191)
- Added share data in news items serialization (nextcloud#1191)
- Added tests for the news items share feature (nextcloud#1191)
- Added sharing articles with nextcloud users (nextcloud#1217)
- Added sharing articles on social media (Facebook, Twitter) or mail (nextcloud#1217)
- Allow installation on Nextcloud v22
- Remove deprecated API endpoints and occ command (nextcloud#935)
  - /api/v1-2/user
  - /api/v1-2/user/avatar
  - ./occ news:updater:all-feeds
- added feed search (nextcloud#1402)

Fixed
- allow calling `/items?getRead=false` without a feed/folder (nextcloud#1380 nextcloud#1356)
- newestId does not return newest ID but last updated (nextcloud#1339)
- removed reference for deleted repair-steps (nextcloud#1399)
- Fix NotNullConstraintViolation when sharing news items with users (nextcloud#1406)

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
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Mark as read" broken since recent updates
4 participants