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

fix(search): fix load more #38303

Merged
merged 1 commit into from
May 16, 2023
Merged

fix(search): fix load more #38303

merged 1 commit into from
May 16, 2023

Conversation

skjnldsv
Copy link
Member

Fix #35558
Regression from #34100

When search for files, clicking the "load more" button, changes the URL and the legacy files list triggers a navigation change.
I fixed the url change and added a sanity check in the legacy url change detection to potentially mitigate other unseen issues.

Signed-off-by: John Molakvoæ <[email protected]>
@skjnldsv
Copy link
Member Author

/backport to stable26

@skjnldsv
Copy link
Member Author

/backport to stable25

Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and works however one small remark

core/src/views/UnifiedSearch.vue Show resolved Hide resolved
Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see above

@skjnldsv skjnldsv added 2. developing Work in progress 3. to review Waiting for reviews and removed 3. to review Waiting for reviews 2. developing Work in progress labels May 16, 2023
@skjnldsv skjnldsv requested a review from artonge May 16, 2023 15:22
@szaimen szaimen merged commit e8cad55 into master May 16, 2023
@szaimen szaimen deleted the fix/loadmore branch May 16, 2023 17:24
@backportbot-nextcloud
Copy link

The backport to stable25 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable25
git pull origin/stable25

# Create the new backport branch
git checkout -b fix/foo-stable25

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123

# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable25

More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport

@szaimen
Copy link
Contributor

szaimen commented May 16, 2023

/backport to stable25

@ostasevych
Copy link

ostasevych commented May 17, 2023

Hi! Please, suggest quick and dirty hack how to get these changes on my personal prod site?

@skjnldsv
Copy link
Member Author

Hi! Please, suggest quick and dirty hack how to get these changes on my personal prod site?

You can download the patch and manually try to apply it from either the stable25 or stable26 branch

https://coderwall.com/p/6aw72a/creating-patch-from-github-pull-request

@blizzz blizzz mentioned this pull request May 17, 2023
@t-markmann
Copy link

Hi! Please, suggest quick and dirty hack how to get these changes on my personal prod site?

You can download the patch and manually try to apply it from either the stable25 or stable26 branch

* [[stable25] fix(search): fix load more #38325](https://github.com/nextcloud/server/pull/38325)

* [[stable26] fix(search): fix load more #38318](https://github.com/nextcloud/server/pull/38318)

https://coderwall.com/p/6aw72a/creating-patch-from-github-pull-request

Unfortunately this only seems to work, if your (production) NC instance is a git repo.
If you execute the .patch in a normally installed NC directory, this will be returned: fatal: not a git repository (or any parent up to mount point /)

Solution might be "git init" and so on. But then your NC dir ist not clean and upgradeable anymore, I think.

A more practicable solution: download the 3 (non-src) files and replace the old ones.
These commands will do (if you adjust your www-data user and the nc path):

NCBASE=/var/www/nextcloud

cd ${NCBASE}/apps/files/js
sudo -u www-data curl -O https://raw.githubusercontent.com/nextcloud/server/b4d7f56b7adb5f74b243f3520b774451158c57d5/apps/files/js/app.js

cd ${NCBASE}/dist
sudo -u www-data curl -O https://raw.githubusercontent.com/nextcloud/server/b4d7f56b7adb5f74b243f3520b774451158c57d5/dist/core-unified-search.js

sudo -u www-data curl -O https://raw.githubusercontent.com/nextcloud/server/b4d7f56b7adb5f74b243f3520b774451158c57d5/dist/core-unified-search.js.map

@ostasevych
Copy link

ostasevych commented May 23, 2023

Hi! Please, suggest quick and dirty hack how to get these changes on my personal prod site?

You can download the patch and manually try to apply it from either the stable25 or stable26 branch

* [[stable25] fix(search): fix load more #38325](https://github.com/nextcloud/server/pull/38325)

* [[stable26] fix(search): fix load more #38318](https://github.com/nextcloud/server/pull/38318)

https://coderwall.com/p/6aw72a/creating-patch-from-github-pull-request

Unfortunately this only seems to work, if your (production) NC instance is a git repo. If you execute the .patch in a normally installed NC directory, this will be returned: fatal: not a git repository (or any parent up to mount point /)

Solution might be "git init" and so on. But then your NC dir ist not clean and upgradeable anymore, I think.

A more practicable solution: download the 3 (non-src) files and replace the old ones. These commands will do (if you adjust your www-data user and the nc path):

NCBASE=/var/www/nextcloud

cd ${NCBASE}/apps/files/js
sudo -u www-data curl -O https://raw.githubusercontent.com/nextcloud/server/b4d7f56b7adb5f74b243f3520b774451158c57d5/apps/files/js/app.js

cd ${NCBASE}/dist
sudo -u www-data curl -O https://raw.githubusercontent.com/nextcloud/server/b4d7f56b7adb5f74b243f3520b774451158c57d5/dist/core-unified-search.js

sudo -u www-data curl -O https://raw.githubusercontent.com/nextcloud/server/b4d7f56b7adb5f74b243f3520b774451158c57d5/dist/core-unified-search.js.map

Be careful by applying it:

By replacing these 3 files I am obtaining empty home folder with this error in the logs:
TypeError: OCA\Files\Service\TagService::__construct(): Argument #4 ($homeFolder) must be of type OCP\Files\Folder, null given, called in /var/www/html/nextcloud/apps/files/lib/AppInfo/Application.php on line 111 :(

@t-markmann
Copy link

Hm, I'm running a NC 25.0.5.1
Works fine here...

@ostasevych
Copy link

ostasevych commented May 23, 2023

Hm, I'm running a NC 25.0.5.1 Works fine here...

With 26.0.1 it goes not so well. I haven't to revert back.
UPD: Well, the above mentioned error message in the logs, it seems is not connected with this behaviour, as I found it in the earlier logs....

@t-markmann
Copy link

The Files for 25 / 26 are different.
Try these for NC26:

NCBASE=/var/www/nextcloud

cd ${NCBASE}/apps/files/js
sudo -u www-data curl -O https://raw.githubusercontent.com/nextcloud/server/6da7e66dc19696f991968bb36fba0ee3a3982078/apps/files/js/app.js

cd ${NCBASE}/dist
sudo -u www-data curl -O https://raw.githubusercontent.com/nextcloud/server/6da7e66dc19696f991968bb36fba0ee3a3982078/dist/core-unified-search.js

sudo -u www-data curl -O https://raw.githubusercontent.com/nextcloud/server/6da7e66dc19696f991968bb36fba0ee3a3982078/dist/core-unified-search.js.map

@ostasevych
Copy link

The Files for 25 / 26 are different. Try these for NC26:

NCBASE=/var/www/nextcloud

cd ${NCBASE}/apps/files/js
sudo -u www-data curl -O https://raw.githubusercontent.com/nextcloud/server/6da7e66dc19696f991968bb36fba0ee3a3982078/apps/files/js/app.js

cd ${NCBASE}/dist
sudo -u www-data curl -O https://raw.githubusercontent.com/nextcloud/server/6da7e66dc19696f991968bb36fba0ee3a3982078/dist/core-unified-search.js

sudo -u www-data curl -O https://raw.githubusercontent.com/nextcloud/server/6da7e66dc19696f991968bb36fba0ee3a3982078/dist/core-unified-search.js.map

Thanks. That works fine!

@skjnldsv
Copy link
Member Author

Unfortunately this only seems to work, if your (production) NC instance is a git repo.

Yeah, this is not for production.
If you don't know what you're doing, I suggest to not experiment with your production services :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Search results broken when requesting to load more results
5 participants