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

feat: make systemtags public visible #48206

Merged
merged 1 commit into from
Oct 15, 2024
Merged

feat: make systemtags public visible #48206

merged 1 commit into from
Oct 15, 2024

Conversation

skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Sep 19, 2024 •
edited
Loading

fix #40364

image

Of course if the tag is made invisible in the settings, it will also apply.
I'm also thinking if it would make sense to NOT show restricted tags on public
image

@skjnldsv skjnldsv added this to the Nextcloud 31 milestone Sep 19, 2024
@skjnldsv skjnldsv self-assigned this Sep 19, 2024
@skjnldsv skjnldsv requested review from susnux, artonge, a team, ArtificialOwl and sorbaugh and removed request for a team September 19, 2024 12:50
susnux
susnux reviewed Sep 19, 2024
View reviewed changes
lib/private/Tags.php Outdated Show resolved Hide resolved
@skjnldsv skjnldsv force-pushed the feat/systemtags-public branch from 7244306 to e3a0f90 Compare September 19, 2024 13:05
github-advanced-security[bot]
github-advanced-security bot found potential problems Sep 19, 2024
View reviewed changes
apps/systemtags/lib/Listeners/BeforeTemplateRenderedListener.php Fixed Show resolved Hide resolved
apps/systemtags/lib/Listeners/LoadAdditionalScriptsListener.php
if (!$event instanceof LoadAdditionalScriptsEvent) {
return;
}
Util::addInitScript(Application::APP_ID, 'init');

Check failure

Code scanning / Psalm

InvalidClass

Class, interface or enum OCA\Systemtags\AppInfo\Application has wrong casing
@skjnldsv skjnldsv force-pushed the feat/systemtags-public branch 3 times, most recently from ba84f93 to 33403e1 Compare September 19, 2024 14:04
@skjnldsv skjnldsv force-pushed the feat/systemtags-public branch from 663268a to 9d535f6 Compare September 19, 2024 17:52
susnux
susnux reviewed Sep 20, 2024
View reviewed changes
apps/dav/lib/Server.php Outdated
@@ -197,7 +197,7 @@ public function __construct(IRequest $request, string $baseUri) {
}

// system tags plugins
$this->server->addPlugin(\OC::$server->get(SystemTagPlugin::class));
// $this->server->addPlugin(\OCP\Server::get(SystemTagPlugin::class));
Copy link
Contributor

Choose a reason for hiding this comment

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

?

Copy link
Member Author

Choose a reason for hiding this comment

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

Added in the apps/systemtags/lib/Listeners/BeforeSabrePubliclyLoadedListener.php now :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Withdrawn, this event is for public only 🤦

Copy link
Contributor

Choose a reason for hiding this comment

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

What about SabrePluginAddEvent?

@skjnldsv skjnldsv force-pushed the feat/systemtags-public branch from 9d535f6 to 6973ce7 Compare October 8, 2024 16:52
@skjnldsv skjnldsv requested a review from susnux October 8, 2024 16:52
@skjnldsv
Copy link
Member Author

skjnldsv commented Oct 8, 2024

Done @susnux

@nickvergessen
Copy link
Member

I'm also thinking if it would make sense to NOT show restricted tags on public

There are 2 flags: can assign, can see. Restricted is false, true. So would be okay to show from that perspective. But I can see how guests add a new angle to it.
I would still show them and instead add a "Show tags on public pages" appconfig?

@skjnldsv
Copy link
Member Author

skjnldsv commented Oct 9, 2024

"Show tags on public pages" appconfig?

I'd prefer to avoid adding yet another config 😭
Since we have public, I think it's straightforward enough to show it in public 🤷

@skjnldsv skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Oct 9, 2024
@skjnldsv skjnldsv force-pushed the feat/systemtags-public branch from 6973ce7 to f70dbe7 Compare October 10, 2024 07:16
@skjnldsv
feat: make systemtags public visible
abd0cdd 
Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>
@skjnldsv skjnldsv force-pushed the feat/systemtags-public branch from f70dbe7 to abd0cdd Compare October 11, 2024 14:06
@skjnldsv skjnldsv merged commit 832695d into master Oct 15, 2024
177 checks passed
@skjnldsv skjnldsv deleted the feat/systemtags-public branch October 15, 2024 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@come-nc come-nc come-nc approved these changes

@provokateurin provokateurin provokateurin approved these changes

@artonge artonge Awaiting requested review from artonge

@ArtificialOwl ArtificialOwl Awaiting requested review from ArtificialOwl ArtificialOwl was automatically assigned from nextcloud/server-backend

@sorbaugh sorbaugh Awaiting requested review from sorbaugh sorbaugh was automatically assigned from nextcloud/server-backend

@nickvergessen nickvergessen Awaiting requested review from nickvergessen

@icewind1991 icewind1991 Awaiting requested review from icewind1991

@susnux susnux Awaiting requested review from susnux

Assignees

@skjnldsv skjnldsv

Labels
2. developing Work in progress enhancement feature: tags
Projects
None yet
Milestone
Nextcloud 31
Development

Successfully merging this pull request may close these issues.

Show tags in public shares
5 participants
@skjnldsv @nickvergessen @susnux @provokateurin @come-nc