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

Public page shares #1041

Merged
merged 26 commits into from
Dec 20, 2023
Merged

Public page shares #1041

merged 26 commits into from
Dec 20, 2023

Conversation

mejo-
Copy link
Member

@mejo- mejo- commented Dec 6, 2023

📝 Summary

🖼️ Screenshots

🏚️ Collective actions before 🏡 Collective actions after
grafik grafik
'Share' sidebar tab Configure share permissions
grafik grafik
Screencast (slightly outdated)
recording1

🚧 TODO

  • backend:
    • share parent folder if pageId is index page
    • implement checkPagePermissions() in PublicPageController
    • implement `findChildren() in PageService
    • add support for single-page share
    • check consequences of adding subpage in single-page shares
    • allow to create several shares
    • make sure move/copy page cannot break out of share realm
  • frontend:
    • add share tab in page sidebar
    • add options to edit share permissions
    • move share collective to landingpage page sidebar
    • add link pointing to sidebar to collective actions
  • UI adjustments
    • don't show trash
    • hide 'delete page' options in UI
    • adjust move/copy page picker
  • make sure mountpoint works
  • behat tests:
    • add/get/delete single-page share
    • fix attachment check
    • add/get/delete page folder share
    • fail to trash page in page share
    • fail to get/create/touch/move/setEmoji/setSubpageOrder/getAttachments/getBacklinks page outside page share
  • cypress tests:
    • create read-only collective share, open (read-only)
    • update collective share to editable, open, edit
    • delete collective share
    • create read-only page share, open (read-only)
    • update page share to editable
    • delete page share

🏁 Checklist

  • Code is properly formatted (npm run lint / npm run stylelint / composer run cs:check)
  • Sign-off message is added to all commits
  • Tests (unit, integration and/or end-to-end) passing and the changes are covered with tests
  • Documentation is not required

@mejo- mejo- added enhancement New feature or request 2. developing labels Dec 6, 2023
Copy link

cypress bot commented Dec 6, 2023

Passing run #1318 ↗︎

0 268 0 0 Flakiness 0

Details:

Public page shares
Project: Collectives Commit: e5c2985850
Status: Passed Duration: 06:26 💡
Started: Dec 20, 2023 11:29 AM Ended: Dec 20, 2023 11:36 AM

Review all test suite changes for PR #1041 ↗︎

@mejo- mejo- force-pushed the feat/page_shares branch 20 times, most recently from dd35114 to d7d8426 Compare December 13, 2023 15:38
@mejo-
Copy link
Member Author

mejo- commented Dec 13, 2023

  • add support for single-page share

This is about sharing pages that don't have a subpage yet. Unfortunately it's not so easy given that right now pages without a subpage are single markdown files (e.g. page.md) and not inside a folder (e.g. page/Readme.md). This leads to problems when sharing the page without subpages and afterwards creating a subpage of it. The markdown file will be replaced and the underlying file share will no longer be valid.

I think we should just do the following:

This is a first step in migrating away from the mix of page.md and page/Readme.md to only use the second format, which we discussed earlier and I consider a good thing in terms of consistency anyway (e.g. attachments then always reside in the page folder, not in the parent page folder).

What do you think @max-nextcloud?

@max-nextcloud
Copy link
Collaborator

What do you think @max-nextcloud?

@mejo- sounds like a good approach to me.

(e.g. attachments then always reside in the page folder, not in the parent page folder).

I thought they'd always live in a .attachments.${fileId} folder. But i guess that folder is either in the parent folder or in the same folder as the file itself (Readme.md)

@mejo-
Copy link
Member Author

mejo- commented Dec 13, 2023

I thought they'd always live in a .attachments.${fileId} folder. But i guess that folder is either in the parent folder or in the same folder as the file itself (Readme.md)

Exactly 😉

@nimishavijay
Copy link
Member

nimishavijay commented Dec 13, 2023

Super nice improvement! It looks great, only 2 small changes:

  • use a different icon for the copy button. My suggestion is the copy MDI icon.
  • unsure about wording "Manage share links" maybe just "Share" works?

This is maybe a bigger discussion, but I wonder if it makes sense to combine the "Manage members" and "Manage sharing" to some degree. Both of those things are regarding who has access to the collective/page. This could be as simple as showing the members of the collective in the sharing tab as a "People with access" section (similar to the "Others with access" in the Files sharing) where there is also a button to "Manage members". But that's definitely a follow-up topic :)

* Hide list of collectives
* Show root page emoji+title instead of collective emoji+name

Signed-off-by: Jonas <[email protected]>
We no longer change `page/Readme.md` back to `page.md` if `page` becomes
a leaf page (i.e. without children). In the long term we want to move to
the `page/Readme.md` format everywhere.

Signed-off-by: Jonas <[email protected]>
Fixes opening shares if there's more than one for a collective.

Signed-off-by: Jonas <[email protected]>
@mejo- mejo- force-pushed the feat/page_shares branch 3 times, most recently from 71d234a to 0916700 Compare December 20, 2023 11:21
@mejo- mejo- merged commit 5b4201c into main Dec 20, 2023
57 checks passed
@delete-merged-branch delete-merged-branch bot deleted the feat/page_shares branch December 20, 2023 11:40
@punkyard
Copy link

hi,
and could you add shares for each page? both public and internal shares in drop down menu, please
happy new year!

@gymnae
Copy link

gymnae commented Feb 5, 2024

Second that question: The mockups in OP show a nice single page sharing dialogue, but it appears absent in the version shipped with NC 28

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow multiple share links with different edit rights Add support for public shares of pages
5 participants