Skip to content

Commit a109b4b

Browse files
rayvincent2rayvincent2
committed
feat(settings): drop failures UI and align Previews admin with review feedback
Remove preview-failure recording from this PR. Convert the admin API to an OCS controller with ApiRoute attributes, save on edit, keep sibling NcSettingsSection blocks, and keep provider table order when toggling. Assisted-by: Grok:grok-4.6 Signed-off-by: Ray Vincent <rayhvincent@gmail.com>
1 parent fda7774 commit a109b4b

22 files changed

Lines changed: 1171 additions & 2161 deletions

File tree

‎apps/files_sharing/lib/Controller/PublicPreviewController.php‎

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
namespace OCA\Files_Sharing\Controller;
99

10-
use OC\Preview\Failure\PreviewFailureService;
1110
use OCP\AppFramework\Http;
1211
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
1312
use OCP\AppFramework\Http\Attribute\NoSameSiteCookieRequired;
@@ -41,7 +40,6 @@ public function __construct(
4140
ISession $session,
4241
private IPreview $previewManager,
4342
private IMimeIconProvider $mimeIconProvider,
44-
private ?PreviewFailureService $failureService = null,
4543
) {
4644
parent::__construct($appName, $request, $session);
4745
}
@@ -154,19 +152,13 @@ public function getPreview(
154152

155153
$response->cacheFor($cacheForSeconds);
156154
return $response;
157-
} catch (NotFoundException $e) {
155+
} catch (NotFoundException) {
158156
// If a preview could not be generated for a resolved file, we can redirect to the mime icon if any
159157
if ($mimeFallback && $previewFile instanceof File) {
160158
if ($url = $this->mimeIconProvider->getMimeIconUrl($previewFile->getMimeType())) {
161159
return new RedirectResponse($url);
162160
}
163161
}
164-
if ($previewFile instanceof File) {
165-
$this->failureService?->recordFromFailedRequest(
166-
$previewFile,
167-
$e->getMessage() !== '' ? $e->getMessage() : 'Preview not found',
168-
);
169-
}
170162
return new DataResponse([], Http::STATUS_NOT_FOUND);
171163
} catch (NotPermittedException) {
172164
return new DataResponse([], Http::STATUS_FORBIDDEN);
@@ -217,7 +209,6 @@ public function directLink(string $token) {
217209
return new DataResponse([], Http::STATUS_FORBIDDEN);
218210
}
219211

220-
$node = null;
221212
try {
222213
$node = $share->getNode();
223214
if ($node instanceof Folder) {
@@ -229,19 +220,12 @@ public function directLink(string $token) {
229220
$response = new FileDisplayResponse($f, Http::STATUS_OK, ['Content-Type' => $f->getMimeType()]);
230221
$response->cacheFor(3600 * 24);
231222
return $response;
232-
} catch (NotFoundException $e) {
233-
if ($node instanceof File) {
234-
$this->failureService?->recordFromFailedRequest(
235-
$node,
236-
$e->getMessage() !== '' ? $e->getMessage() : 'Preview not found',
237-
);
238-
}
223+
} catch (NotFoundException) {
239224
return new DataResponse([], Http::STATUS_NOT_FOUND);
240225
} catch (NotPermittedException) {
241226
return new DataResponse([], Http::STATUS_FORBIDDEN);
242227
} catch (\InvalidArgumentException $e) {
243228
return new DataResponse([], Http::STATUS_BAD_REQUEST);
244229
}
245230
}
246-
247231
}

‎apps/files_trashbin/lib/Controller/PreviewController.php‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99

1010
namespace OCA\Files_Trashbin\Controller;
1111

12-
use OC\Preview\Failure\PreviewFailureService;
1312
use OCA\Files_Trashbin\Trash\ITrashManager;
1413
use OCP\AppFramework\Controller;
1514
use OCP\AppFramework\Http;
@@ -38,7 +37,6 @@ public function __construct(
3837
private IMimeTypeDetector $mimeTypeDetector,
3938
private IPreview $previewManager,
4039
private ITimeFactory $time,
41-
private ?PreviewFailureService $failureService = null,
4240
) {
4341
parent::__construct($appName, $request);
4442
}
@@ -69,7 +67,6 @@ public function getPreview(
6967
return new DataResponse([], Http::STATUS_BAD_REQUEST);
7068
}
7169

72-
$file = null;
7370
try {
7471
$file = $this->trashManager->getTrashNodeById($this->userSession->getUser(), $fileId);
7572
if ($file === null) {
@@ -99,12 +96,6 @@ public function getPreview(
9996
$response->cacheFor(3600 * 24);
10097
return $response;
10198
} catch (NotFoundException $e) {
102-
if ($file instanceof \OCP\Files\File) {
103-
$this->failureService?->recordFromFailedRequest(
104-
$file,
105-
$e->getMessage() !== '' ? $e->getMessage() : 'Preview not found',
106-
);
107-
}
10899
return new DataResponse([], Http::STATUS_NOT_FOUND);
109100
} catch (\InvalidArgumentException $e) {
110101
return new DataResponse([], Http::STATUS_BAD_REQUEST);

‎apps/files_versions/lib/Controller/PreviewController.php‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
namespace OCA\Files_Versions\Controller;
99

10-
use OC\Preview\Failure\PreviewFailureService;
1110
use OCA\Files_Versions\Versions\IVersionManager;
1211
use OCP\AppFramework\Controller;
1312
use OCP\AppFramework\Http;
@@ -35,7 +34,6 @@ public function __construct(
3534
private IVersionManager $versionManager,
3635
private IPreview $previewManager,
3736
private IMimeIconProvider $mimeIconProvider,
38-
private ?PreviewFailureService $failureService = null,
3937
) {
4038
parent::__construct($appName, $request);
4139
}
@@ -87,12 +85,6 @@ public function getPreview(
8785
}
8886
}
8987

90-
if ($versionFile instanceof \OCP\Files\File) {
91-
$this->failureService?->recordFromFailedRequest(
92-
$versionFile,
93-
$e->getMessage() !== '' ? $e->getMessage() : 'Preview not found',
94-
);
95-
}
9688
return new DataResponse([], Http::STATUS_NOT_FOUND);
9789
} catch (\InvalidArgumentException $e) {
9890
return new DataResponse([], Http::STATUS_BAD_REQUEST);

‎apps/settings/appinfo/routes.php‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,6 @@
4141
['name' => 'TwoFactorSettings#index', 'url' => '/settings/api/admin/twofactorauth', 'verb' => 'GET' , 'root' => ''],
4242
['name' => 'TwoFactorSettings#update', 'url' => '/settings/api/admin/twofactorauth', 'verb' => 'PUT' , 'root' => ''],
4343
['name' => 'AISettings#update', 'url' => '/settings/api/admin/ai', 'verb' => 'PUT' , 'root' => ''],
44-
['name' => 'PreviewAdmin#update', 'url' => '/settings/api/admin/previews', 'verb' => 'PUT' , 'root' => ''],
45-
['name' => 'PreviewAdmin#testImaginary', 'url' => '/settings/api/admin/previews/imaginary/test', 'verb' => 'POST' , 'root' => ''],
46-
['name' => 'PreviewAdmin#listFailures', 'url' => '/settings/api/admin/previews/failures', 'verb' => 'GET' , 'root' => ''],
47-
['name' => 'PreviewAdmin#retryFailure', 'url' => '/settings/api/admin/previews/failures/{id}/retry', 'verb' => 'POST' , 'root' => ''],
48-
['name' => 'PreviewAdmin#deleteFailure', 'url' => '/settings/api/admin/previews/failures/{id}', 'verb' => 'DELETE' , 'root' => ''],
49-
['name' => 'PreviewAdmin#clearFailures', 'url' => '/settings/api/admin/previews/failures', 'verb' => 'DELETE' , 'root' => ''],
5044

5145
['name' => 'Preset#getPreset', 'url' => '/settings/preset', 'verb' => 'GET' , 'root' => ''],
5246
['name' => 'Preset#getCurrentPreset', 'url' => '/settings/preset/current', 'verb' => 'GET' , 'root' => ''],

‎apps/settings/lib/Controller/PreviewAdminController.php‎

Lines changed: 27 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -9,61 +9,71 @@
99

1010
namespace OCA\Settings\Controller;
1111

12-
use OC\Preview\Failure\PreviewFailureService;
1312
use OC\Preview\PreviewAdminConfig;
1413
use OCA\Settings\Settings\Admin\Previews;
15-
use OCP\AppFramework\Controller;
16-
use OCP\AppFramework\Db\DoesNotExistException;
1714
use OCP\AppFramework\Http;
15+
use OCP\AppFramework\Http\Attribute\ApiRoute;
1816
use OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting;
17+
use OCP\AppFramework\Http\Attribute\OpenAPI;
1918
use OCP\AppFramework\Http\Attribute\PasswordConfirmationRequired;
2019
use OCP\AppFramework\Http\DataResponse;
21-
use OCP\Files\File;
22-
use OCP\Files\IRootFolder;
23-
use OCP\Files\NotFoundException;
20+
use OCP\AppFramework\OCS\OCSBadRequestException;
21+
use OCP\AppFramework\OCSController;
2422
use OCP\Http\Client\IClientService;
25-
use OCP\IPreview;
2623
use OCP\IRequest;
2724
use Psr\Log\LoggerInterface;
2825

29-
class PreviewAdminController extends Controller {
26+
#[OpenAPI(scope: OpenAPI::SCOPE_ADMINISTRATION)]
27+
class PreviewAdminController extends OCSController {
3028
public function __construct(
3129
string $appName,
3230
IRequest $request,
3331
private PreviewAdminConfig $previewAdminConfig,
34-
private PreviewFailureService $failureService,
3532
private IClientService $clientService,
36-
private IPreview $preview,
37-
private IRootFolder $rootFolder,
3833
private LoggerInterface $logger,
3934
) {
4035
parent::__construct($appName, $request);
4136
}
4237

4338
/**
44-
* @param array $settings
39+
* Update preview administration settings
40+
*
41+
* @param array<string, mixed> $settings Preview settings to persist
42+
* @return DataResponse<Http::STATUS_OK, array<string, mixed>, array{}>
43+
* @throws OCSBadRequestException Invalid settings payload
44+
*
45+
* 200: Settings saved
4546
*/
4647
#[AuthorizedAdminSetting(settings: Previews::class)]
4748
#[PasswordConfirmationRequired]
49+
#[ApiRoute(verb: 'PUT', url: '/api/admin/previews')]
4850
public function update(array $settings): DataResponse {
4951
try {
5052
$this->previewAdminConfig->setSettings($settings);
5153
} catch (\InvalidArgumentException $e) {
52-
return new DataResponse(['error' => $e->getMessage()], Http::STATUS_BAD_REQUEST);
54+
throw new OCSBadRequestException($e->getMessage());
5355
}
5456

5557
return new DataResponse($this->previewAdminConfig->getSettings());
5658
}
5759

60+
/**
61+
* Test connectivity to an Imaginary preview service
62+
*
63+
* @param string|null $url Imaginary base URL
64+
* @param string|null $key Optional Imaginary API key
65+
* @return DataResponse<Http::STATUS_OK, array{status: string, httpCode?: int, error?: string}, array{}>
66+
* @throws OCSBadRequestException Invalid URL
67+
*
68+
* 200: Connection test completed
69+
*/
5870
#[AuthorizedAdminSetting(settings: Previews::class)]
71+
#[ApiRoute(verb: 'POST', url: '/api/admin/previews/imaginary/test')]
5972
public function testImaginary(?string $url = null, ?string $key = null): DataResponse {
6073
try {
6174
$target = $this->previewAdminConfig->validateImaginaryUrl($url ?? '');
6275
} catch (\InvalidArgumentException $e) {
63-
return new DataResponse([
64-
'status' => 'unreachable',
65-
'error' => $e->getMessage(),
66-
], Http::STATUS_BAD_REQUEST);
76+
throw new OCSBadRequestException($e->getMessage());
6777
}
6878

6979
if ($target === '') {
@@ -99,69 +109,4 @@ public function testImaginary(?string $url = null, ?string $key = null): DataRes
99109
]);
100110
}
101111
}
102-
103-
#[AuthorizedAdminSetting(settings: Previews::class)]
104-
public function listFailures(?string $mime = null, ?string $provider = null, ?string $range = null): DataResponse {
105-
$since = $this->rangeToSince($range);
106-
return new DataResponse([
107-
'failures' => $this->failureService->listFailures($mime, $provider, $since),
108-
]);
109-
}
110-
111-
#[AuthorizedAdminSetting(settings: Previews::class)]
112-
public function retryFailure(int $id): DataResponse {
113-
try {
114-
$failure = $this->failureService->get($id);
115-
} catch (DoesNotExistException) {
116-
return new DataResponse(['error' => 'Unknown failure'], Http::STATUS_NOT_FOUND);
117-
}
118-
119-
$nodes = $this->rootFolder->getById($failure->getFileId());
120-
$file = null;
121-
foreach ($nodes as $node) {
122-
if ($node instanceof File) {
123-
$file = $node;
124-
break;
125-
}
126-
}
127-
if ($file === null) {
128-
return new DataResponse(['error' => 'File not found'], Http::STATUS_NOT_FOUND);
129-
}
130-
131-
try {
132-
$this->preview->getPreview($file);
133-
$this->failureService->clearForFile($failure->getFileId());
134-
return new DataResponse(['status' => 'ok']);
135-
} catch (NotFoundException|\InvalidArgumentException $e) {
136-
return new DataResponse([
137-
'status' => 'failed',
138-
'error' => $e->getMessage(),
139-
], Http::STATUS_BAD_REQUEST);
140-
}
141-
}
142-
143-
#[AuthorizedAdminSetting(settings: Previews::class)]
144-
public function deleteFailure(int $id): DataResponse {
145-
try {
146-
$this->failureService->delete($id);
147-
} catch (DoesNotExistException) {
148-
return new DataResponse(['error' => 'Unknown failure'], Http::STATUS_NOT_FOUND);
149-
}
150-
return new DataResponse(['status' => 'ok']);
151-
}
152-
153-
#[AuthorizedAdminSetting(settings: Previews::class)]
154-
public function clearFailures(): DataResponse {
155-
$this->failureService->clearAll();
156-
return new DataResponse(['status' => 'ok']);
157-
}
158-
159-
private function rangeToSince(?string $range): ?int {
160-
return match ($range) {
161-
'24h' => time() - 86400,
162-
'7d' => time() - 7 * 86400,
163-
'30d' => time() - 30 * 86400,
164-
default => null,
165-
};
166-
}
167112
}

‎apps/settings/lib/Settings/Admin/Previews.php‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99

1010
namespace OCA\Settings\Settings\Admin;
1111

12-
use OC\Preview\Failure\PreviewFailureService;
1312
use OC\Preview\PreviewAdminConfig;
1413
use OCP\AppFramework\Http\TemplateResponse;
1514
use OCP\AppFramework\Services\IInitialState;
@@ -19,7 +18,6 @@
1918
class Previews implements IDelegatedSettings {
2019
public function __construct(
2120
private PreviewAdminConfig $previewAdminConfig,
22-
private PreviewFailureService $failureService,
2321
private IInitialState $initialState,
2422
private string $appName,
2523
) {
@@ -28,7 +26,6 @@ public function __construct(
2826
#[\Override]
2927
public function getForm(): TemplateResponse {
3028
$this->initialState->provideInitialState('previewsSettings', $this->previewAdminConfig->getSettings());
31-
$this->initialState->provideInitialState('previewsFailures', $this->failureService->listFailures());
3229
$this->initialState->provideInitialState('previewsDocumentation', 'https://docs.nextcloud.com/server/latest/admin_manual/configuration_files/previews_configuration.html');
3330

3431
Util::addScript($this->appName, 'vue-settings-admin-previews');

0 commit comments

Comments
 (0)