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

Allow to register public share template provider #35736

Merged
merged 1 commit into from
Feb 2, 2023
Merged

Allow to register public share template provider #35736

merged 1 commit into from
Feb 2, 2023

Conversation

CarlSchwan
Copy link
Member

Signed-off-by: Carl Schwan [email protected]

github-advanced-security[bot]
github-advanced-security bot found potential problems Dec 12, 2022
View reviewed changes
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

Psalm found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.

@CarlSchwan CarlSchwan marked this pull request as draft December 12, 2022 12:14
github-advanced-security[bot]
github-advanced-security bot found potential problems Dec 12, 2022
View reviewed changes
apps/files_sharing/lib/DefaultShareDisplayTemplateProvider.php Fixed Show fixed Hide fixed
apps/files_sharing/lib/DefaultShareDisplayTemplateProvider.php Fixed Show fixed Hide fixed
lib/private/Share20/ShareDisplayTemplateFactory.php Fixed Show fixed Hide fixed
apps/files_sharing/lib/Controller/ShareController.php
$template = $this->shareDisplayTemplateFactory->getTemplate($share);
$response = $template->renderPage($share, $this->getToken(), $path);
} catch (NotFoundException $e) {
$this->emitAccessShareHook($share, 404, 'Share not found');

Check notice

Code scanning / Psalm

DeprecatedMethod

The method OCA\Files_Sharing\Controller\ShareController::emitAccessShareHook has been marked as deprecated
apps/files_sharing/lib/DefaultShareDisplayTemplateProvider.php Fixed Show fixed Hide fixed
apps/files_sharing/lib/DefaultShareDisplayTemplateProvider.php Fixed Show fixed Hide fixed
apps/files_sharing/lib/DefaultShareDisplayTemplateProvider.php Fixed Show fixed Hide fixed
apps/files_sharing/lib/DefaultShareDisplayTemplateProvider.php Fixed Show fixed Hide fixed
@szaimen szaimen added this to the Nextcloud 26 milestone Dec 12, 2022
@szaimen szaimen added the 2. developing Work in progress label Dec 12, 2022
@tobiasKaminsky tobiasKaminsky mentioned this pull request Dec 19, 2022
Closed
3 tasks
@artonge artonge force-pushed the pluggable-share-display branch 2 times, most recently from 6e7eb80 to e5a0fff Compare December 19, 2022 14:17
github-advanced-security[bot]
github-advanced-security bot found potential problems Dec 22, 2022
View reviewed changes
apps/files_sharing/lib/Controller/ShareController.php
IConfig $config,
IURLGenerator $urlGenerator,
IUserManager $userManager,
ILogger $logger,

Check notice

Code scanning / Psalm

DeprecatedClass

Class OCP\ILogger is marked as deprecated
@artonge artonge marked this pull request as ready for review December 27, 2022 10:22
@artonge artonge mentioned this pull request Jan 2, 2023
Merged
16 tasks
@artonge artonge self-assigned this Jan 3, 2023
@artonge artonge added 3. to review Waiting for reviews enhancement feature: sharing php Pull requests that update Php code and removed 2. developing Work in progress labels Jan 3, 2023
@tobiasKaminsky
Copy link
Member

/rebase

artonge
artonge reviewed Jan 18, 2023
View reviewed changes
apps/files/css/files.scss Outdated Show resolved Hide resolved
@artonge artonge changed the title Pluggable share provider Pluggable public share template provider Jan 18, 2023
@artonge artonge force-pushed the pluggable-share-display branch 4 times, most recently from 64f0172 to a7ccca4 Compare January 18, 2023 15:47
github-advanced-security[bot]
github-advanced-security bot found potential problems Jan 31, 2023
View reviewed changes
apps/files_sharing/lib/DefaultPublicShareTemplateProvider.php
$shareTmpl['dirToken'] = $token;
$shareTmpl['sharingToken'] = $token;
$shareTmpl['server2serversharing'] = $this->federatedShareProvider->isOutgoingServer2serverShareEnabled();
$shareTmpl['protected'] = $share->getPassword() !== null ? 'true' : 'false';

Check notice

Code scanning / Psalm

RedundantConditionGivenDocblockType

Docblock-defined type string can never contain null
apps/files_sharing/lib/DefaultPublicShareTemplateProvider.php Fixed Show resolved Hide resolved
apps/files_sharing/lib/DefaultPublicShareTemplateProvider.php

$folder = new Template('files', 'list', '');

$folder->assign('dir', $shareNode->getRelativePath($folderNode->getPath()));

Check notice

Code scanning / Psalm

PossiblyNullArgument

Argument 2 of OCP\Template::assign cannot be null, possibly null value provided
apps/files_sharing/lib/DefaultPublicShareTemplateProvider.php Fixed Show fixed Hide fixed
@artonge artonge requested a review from come-nc January 31, 2023 09:59
@blizzz blizzz mentioned this pull request Feb 1, 2023
Merged
come-nc
come-nc approved these changes Feb 2, 2023
View reviewed changes
Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

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

Good apart from psalm reported issues I highlighted.

@blizzz
Copy link
Member

blizzz commented Feb 2, 2023

The autoloaders are not up to date
Please run: bash build/autoloaderchecker.sh
And commit the result

@artonge
Pluggable share provider
4ab3c16 
Signed-off-by: Carl Schwan <[email protected]>
Signed-off-by: Louis Chemineau <[email protected]>
@blizzz blizzz disabled auto-merge February 2, 2023 16:33
@blizzz
Copy link
Member

blizzz commented Feb 2, 2023

You shall not merge with red CI… but, hey, perhaps I get more funny Microsoft Github badges.

@blizzz blizzz merged commit d83ea28 into master Feb 2, 2023
@blizzz blizzz deleted the pluggable-share-display branch February 2, 2023 16:33
@nickvergessen
Copy link
Member

You shall not merge with red CI… but, hey, perhaps I get more funny Microsoft Github badges.

Actually the red CI is caused by this:
grafik

grafik

Before After
Bildschirmfoto vom 2023-02-03 09-38-08 Bildschirmfoto vom 2023-02-03 09-37-34 
# status: waiting for both good and bad commits
# good: [be1de30a4f6b37da7c710ac55489e4938c4ded91] Fix(l10n): 🔠 Update translations from Transifex
git bisect good be1de30a4f6b37da7c710ac55489e4938c4ded91
# status: waiting for bad commit, 1 good commit known
# bad: [94767112dd4210483394b24d4fdaddea92f0dc8b] Fix(l10n): 🔠 Update translations from Transifex
git bisect bad 94767112dd4210483394b24d4fdaddea92f0dc8b
# good: [e4fadcf02a1146bc454e0217b5883dd2451ed96f] Merge pull request #36434 from nextcloud/techdebt/noid/drop-bootstrap
git bisect good e4fadcf02a1146bc454e0217b5883dd2451ed96f
# good: [fc4e87a2dfc5ff53bc9f15da13f355dd285769a9] Merge pull request #36487 from nextcloud/bugfix/noid/fix-query-builder-usage-in-dav-account-deletion
git bisect good fc4e87a2dfc5ff53bc9f15da13f355dd285769a9
# good: [38e9cb6a052f4d0bc4e34293febbe9f132055a83] Use recurrence instance to build iMip email
git bisect good 38e9cb6a052f4d0bc4e34293febbe9f132055a83
# bad: [39f202581bc72209f2f9c138701281267ba30d39] Merge pull request #36497 from nextcloud/techdebt/noid/fix-autotest-execution
git bisect bad 39f202581bc72209f2f9c138701281267ba30d39
# bad: [d83ea282c4bddd332623c3c0a8c4dea01146047f] Merge pull request #35736 from nextcloud/pluggable-share-display
git bisect bad d83ea282c4bddd332623c3c0a8c4dea01146047f
# good: [6f3c4f2255ed73601fa5eac3048d679caa3065e0] Merge pull request #36450 from nextcloud/fix/clipboard-copy
git bisect good 6f3c4f2255ed73601fa5eac3048d679caa3065e0
# bad: [4ab3c16403e69811cf2353ba2bfeafcbcf259c42] Pluggable share provider
git bisect bad 4ab3c16403e69811cf2353ba2bfeafcbcf259c42
# first bad commit: [4ab3c16403e69811cf2353ba2bfeafcbcf259c42] Pluggable share provider

4ab3c16403e69811cf2353ba2bfeafcbcf259c42 is the first bad commit
commit 4ab3c16403e69811cf2353ba2bfeafcbcf259c42
Author: Louis Chemineau <[email protected]>
Date:   Wed Jan 18 16:53:22 2023 +0100

    Pluggable share provider
    
    Signed-off-by: Carl Schwan <[email protected]>
    Signed-off-by: Louis Chemineau <[email protected]>

 .../composer/composer/autoload_classmap.php        |   1 +
 .../composer/composer/autoload_static.php          |   1 +
 .../lib/Controller/ShareController.php             | 250 +++---------------
 .../lib/DefaultPublicShareTemplateProvider.php     | 292 +++++++++++++++++++++
 apps/files_sharing/templates/public.php            |   2 +-
 .../tests/Controller/ShareControllerTest.php       |  25 +-
 lib/composer/composer/autoload_classmap.php        |   3 +
 lib/composer/composer/autoload_static.php          |   3 +
 .../AppFramework/Bootstrap/RegistrationContext.php |  22 ++
 lib/private/Server.php                             |   2 +
 lib/private/Share20/PublicShareTemplateFactory.php |  63 +++++
 .../Bootstrap/IRegistrationContext.php             |  10 +
 lib/public/Share/IPublicShareTemplateFactory.php   |  35 +++
 lib/public/Share/IPublicShareTemplateProvider.php  |  42 +++
 14 files changed, 532 insertions(+), 219 deletions(-)
 create mode 100644 apps/files_sharing/lib/DefaultPublicShareTemplateProvider.php
 create mode 100644 lib/private/Share20/PublicShareTemplateFactory.php
 create mode 100644 lib/public/Share/IPublicShareTemplateFactory.php
 create mode 100644 lib/public/Share/IPublicShareTemplateProvider.php

@nickvergessen nickvergessen mentioned this pull request Feb 3, 2023
Closed
nickvergessen added a commit that referenced this pull request Feb 3, 2023
@nickvergessen
fix(sharing): Remove casting to int from INF
4eb77ff 
Regression from #35736
INF is a the float INF, casting it to integer will make it 0

Signed-off-by: Joas Schilling <[email protected]>
@nickvergessen nickvergessen mentioned this pull request Feb 3, 2023
Merged
4 tasks
nickvergessen added a commit that referenced this pull request Feb 7, 2023
@nickvergessen
fix(sharing): Remove casting to int from INF
80071bd 
Regression from #35736
INF is a the float INF, casting it to integer will make it 0

Signed-off-by: Joas Schilling <[email protected]>
summersab pushed a commit to summersab/server that referenced this pull request Feb 9, 2023
@nickvergessen @summersab
fix(sharing): Remove casting to int from INF
51e4046 
Regression from nextcloud#35736
INF is a the float INF, casting it to integer will make it 0

Signed-off-by: Joas Schilling <[email protected]>
@DaphneMuller
Copy link

hello @CarlSchwan @artonge
Thank you so much for your work on this pull request! This ticket seems to have the tag 'pending documentation'.
Is there any chance you could clarify what documentation is missing? Is this for admins or for app developers?

@artonge
Copy link
Contributor

artonge commented Feb 21, 2023

Forgot about it, here it is: nextcloud/documentation#9647
Thanks for the ping :)

@artonge artonge mentioned this pull request Feb 21, 2023
Merged
@artonge artonge removed the pending documentation This pull request needs an associated documentation update label Feb 21, 2023
@DaphneMuller
Copy link

thank you for your lightning fast reply!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@artonge artonge artonge left review comments

@skjnldsv skjnldsv skjnldsv left review comments

@PVince81 PVince81 PVince81 approved these changes

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

@icewind1991 icewind1991 Awaiting requested review from icewind1991

@ArtificialOwl ArtificialOwl Awaiting requested review from ArtificialOwl

@blizzz blizzz Awaiting requested review from blizzz

Assignees

@artonge artonge

Labels
3. to review Waiting for reviews enhancement feature: sharing php Pull requests that update Php code
Projects
None yet
Milestone
Nextcloud 26
Development

Successfully merging this pull request may close these issues.
10 participants
@CarlSchwan @tobiasKaminsky @artonge @blizzz @nickvergessen @DaphneMuller @PVince81 @skjnldsv @come-nc @szaimen