Add Unified Sharing#62241
Conversation
provokateurin
requested review from
Altahrim and
come-nc
and removed request for
a team
| $registry = Server::get(ISharingRegistry::class); | ||
|
|
||
| $registry->registerSourceType(Server::get(NodeShareSourceType::class)); |
There was a problem hiding this comment.
Looks weird to me that register is pulling instances from Server::get.
It’s usually registering classnames in the context that later gets vivified if needed.
Or you can use injectFn in boot and you get proper DI.
(I do not have context for this, feel free to ignore if not relevant)
| * SPDX-License-Identifier: AGPL-3.0-or-later | ||
| */ | ||
|
|
||
| declare(strict_types=1); |
There was a problem hiding this comment.
declare should be before licenses to match other files style.
| private ?IAppConfig $appConfig = null; | ||
|
|
||
| private function getAppConfig(): IAppConfig { | ||
| return $this->appConfig ??= Server::get(IAppConfig::class); | ||
| } |
There was a problem hiding this comment.
How come this is needed and proper DI cannot be used?
(Precisely because these classes get built in register and not boot?)
Signed-off-by: provokateurin <kate@provokateurin.de>
Signed-off-by: provokateurin <kate@provokateurin.de>
Signed-off-by: provokateurin <kate@provokateurin.de>
Signed-off-by: provokateurin <kate@provokateurin.de>
Signed-off-by: provokateurin <kate@provokateurin.de>
provokateurin
force-pushed
the
unified-sharing
branch
from
1cb4c66 to
5b3870e
Compare
| * @since 35.0.0 | ||
| */ | ||
| public function format(IUserManager $userManager): array { | ||
| $ownerUser = $userManager->get($this->userId); |
There was a problem hiding this comment.
Can this use IUserManager::getExistingUser instead (if the instance is local)?
| * @since 35.0.0 | ||
| */ | ||
| #[Consumable(since: '35.0.0')] | ||
| final readonly class ShareIconSVG { |
There was a problem hiding this comment.
It might make sense to move the icon classes outside the share namespace, so they could be used for non-sharing dynamic icon registration in the future.
| /** @var array<int, array<class-string<IShareRecipientType>, bool>> $shareRecipientTypeClasses */ | ||
| $shareRecipientTypeClasses = []; | ||
| foreach ($chunks as $chunk) { | ||
| $qb = $this->connection->getQueryBuilder(); |
There was a problem hiding this comment.
in most cases, we have already done a query on recipients above when finding share ids, probably worth fetching this data then instead.
5 participants
Summary
#51803
Checklist
3. to review, feature component)stable32)AI (if applicable)