-
Notifications
You must be signed in to change notification settings - Fork 19
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
feat(PageReference): Public page reference lookups #1369
Conversation
1 failed and 2 flaky tests on run #1734 ↗︎
Details:
cypress/e2e/pages.spec.js • 1 failed test • Nextcloud master
pages-links.spec.js • 1 flaky test • Nextcloud master
pages.spec.js • 1 flaky test • Nextcloud master
Review all test suite changes for PR #1369 ↗︎ |
484cf64
to
cb0bb54
Compare
cb0bb54
to
cd09e28
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice to see the tests added - that makes me confident it works.
Only skimmed through the php code though. So if you want a thorough review you'll need to wait for @juliushaertl or so.
Besides one minor suggestion this looks good to me.
cd09e28
to
30b9cd3
Compare
Allow to resolve page references by share token to support lookups from public shares. This adds link previews for links to other pages in the same collective in public shares. It requires the new public reference API from nextcloud/server#46378 and the Text app being built with nextcloud-libraries/nextcloud-vue#5800. Fixes: #1275 Contributes to nextcloud/text#5520 Signed-off-by: Jonas <jonas@freesources.org>
30b9cd3
to
ad14083
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Also from my side
Allow to resolve page references by share token to support lookups from public shares. This adds link previews for links to other pages in the same collective in public shares.
It requires the new public reference API from nextcloud/server#46378 and the Text app being built with nextcloud-libraries/nextcloud-vue#5800.
Fixes: #1275
Contributes to nextcloud/text#5520
🏁 Checklist
npm run lint
/npm run stylelint
/composer run cs:check
)