Skip to content

Commit 8399804

Browse files
committed
fix(Trashbin): Improve UX
- do not wait for trashbin items to load on unrelated routes - show proper loading modal if people are waiting for the trashbin to load - Create a new route that empties the trashbin in one go Signed-off-by: Marcel Klehr <mklehr@gmx.net>
1 parent 7f8ffac commit 8399804

7 files changed

Lines changed: 90 additions & 31 deletions

File tree

lib/BackgroundJobs/EmptyTrashbinJob.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
use OCP\BackgroundJob\TimedJob;
1414

1515
class EmptyTrashbinJob extends TimedJob {
16-
public const BATCH_SIZE = 1000; // 40 items
16+
public const BATCH_SIZE = 1000;
1717
public const INTERVAL = 5 * 60; // 5 minutes
1818
public const TRASHBIN_TTL = 2 * 4 * 4 * 7 * 24 * 60 * 60; // Two months
1919

lib/Controller/FoldersController.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -850,4 +850,24 @@ public function getDeletedFolders(): DataResponse {
850850

851851
return new Http\DataResponse(['status' => 'success', 'data' => $folderItems]);
852852
}
853+
854+
/**
855+
* @throws UnauthenticatedError
856+
*/
857+
#[Http\Attribute\NoAdminRequired]
858+
#[Http\Attribute\NoCSRFRequired]
859+
#[Http\Attribute\PublicPage]
860+
#[Http\Attribute\BruteForceProtection(action: 'removeDeletedFolders')]
861+
#[Http\Attribute\FrontpageRoute(verb: 'DELETE', url: '/public/rest/v2/folder/deleted')]
862+
public function removeDeletedFolders(): DataResponse {
863+
$this->authorizer->setCredentials($this->request);
864+
if ($this->authorizer->getUserId() === null) {
865+
$res = new Http\DataResponse(['status' => 'error', 'data' => ['Unauthorized']], Http::STATUS_FORBIDDEN);
866+
$res->throttle();
867+
return $res;
868+
}
869+
$this->treeMapper->deleteTrashbinItemsForUser($this->authorizer->getUserId());
870+
871+
return new Http\DataResponse(['status' => 'success', 'data' => []]);
872+
}
853873
}

lib/Controller/InternalFoldersController.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,4 +156,10 @@ public function deleteShare(int $shareId): DataResponse {
156156
public function getDeletedFolders(): DataResponse {
157157
return $this->controller->getDeletedFolders();
158158
}
159+
160+
#[NoAdminRequired]
161+
#[FrontpageRoute(verb: 'DELETE', url: '/folder/deleted')]
162+
public function removeDeletedFolders(): DataResponse {
163+
return $this->controller->removeDeletedFolders();
164+
}
159165
}

lib/Db/TreeMapper.php

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1288,7 +1288,7 @@ public function deleteOldTrashbinItems(int $limit, float|int $maxAge): void {
12881288
$qb->where($qb->expr()->neq('type', $qb->createNamedParameter(TreeMapper::TYPE_SHARE, IQueryBuilder::PARAM_STR)));
12891289
$cutoffDate = $this->timeFactory->getDateTime();
12901290
$cutoffDate->modify('- ' . ((string)$maxAge) . ' seconds');
1291-
$qb->andWhere($qb->expr()->lt('soft_deleted_at', $qb->createNamedParameter($cutoffDate, IQueryBuilder::PARAM_DATE)));
1291+
$qb->andWhere($qb->expr()->lt('soft_deleted_at', $qb->createNamedParameter($cutoffDate, IQueryBuilder::PARAM_DATETIME_MUTABLE)));
12921292
$qb->setMaxResults($limit);
12931293
try {
12941294
$result = $qb->executeQuery();
@@ -1301,9 +1301,47 @@ public function deleteOldTrashbinItems(int $limit, float|int $maxAge): void {
13011301
$this->deleteEntry($row['type'], $row['id'], $row['parent_folder']);
13021302
} catch (DoesNotExistException $e) {
13031303
// noop
1304-
} catch (UnsupportedOperation|MultipleObjectsReturnedException $e) {
1304+
} catch (UnsupportedOperation|MultipleObjectsReturnedException|Exception $e) {
13051305
$this->logger->error('Could not delete old trash bin item: ' . var_export($row, true), ['exception' => $e]);
13061306
}
13071307
}
13081308
}
1309+
1310+
/**
1311+
* @param string $userId
1312+
* @return void
1313+
*/
1314+
public function deleteTrashbinItemsForUser(string $userId): void {
1315+
$qb = $this->db->getQueryBuilder();
1316+
$qb->select('t.type', 't.id', 't.parent_folder')->from('bookmarks_tree', 't');
1317+
$qb->where($qb->expr()->neq('type', $qb->createNamedParameter(TreeMapper::TYPE_SHARE, IQueryBuilder::PARAM_STR)));
1318+
$qb->andWhere($qb->expr()->isNotNull('soft_deleted_at'));
1319+
$qb->leftJoin('t', 'bookmarks', 'b', 't.id = b.id and t.type = ' . $qb->createNamedParameter(TreeMapper::TYPE_BOOKMARK));
1320+
$qb->leftJoin('t', 'bookmarks_folders', 'f', 't.id = f.id and t.type = ' . $qb->createNamedParameter(TreeMapper::TYPE_FOLDER));
1321+
$qb->andWhere($qb->expr()->orX(
1322+
$qb->expr()->andX(
1323+
$qb->expr()->eq('t.type', $qb->createNamedParameter(TreeMapper::TYPE_BOOKMARK)),
1324+
$qb->expr()->eq('b.user_id', $qb->createNamedParameter($userId)),
1325+
),
1326+
$qb->expr()->andX(
1327+
$qb->expr()->eq('t.type', $qb->createNamedParameter(TreeMapper::TYPE_FOLDER)),
1328+
$qb->expr()->eq('f.user_id', $qb->createNamedParameter($userId)),
1329+
)
1330+
));
1331+
try {
1332+
$result = $qb->executeQuery();
1333+
} catch (Exception $e) {
1334+
$this->logger->error('Could not query for trash bin items of user ' . $userId, ['exception' => $e]);
1335+
return;
1336+
}
1337+
while ($row = $result->fetch()) {
1338+
try {
1339+
$this->deleteEntry($row['type'], $row['id'], $row['parent_folder']);
1340+
} catch (DoesNotExistException $e) {
1341+
// noop
1342+
} catch (UnsupportedOperation|MultipleObjectsReturnedException|Exception $e) {
1343+
$this->logger->error('Could not delete trash bin item: ' . var_export($row, true), ['exception' => $e]);
1344+
}
1345+
}
1346+
}
13091347
}

src/components/BookmarksList.vue

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ export default {
209209
return this.$store.state.settings.sorting
210210
},
211211
loading() {
212-
return this.$store.state.loading.bookmarks || this.$store.state.loading.folders
212+
return this.$store.state.loading.bookmarks || this.$store.state.loading.folders || (this.$route.name === privateRoutes.TRASHBIN && this.$store.state.loading.deleted_folders)
213213
},
214214
},
215215
methods: {

src/components/LoadingModal.vue

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
</template>
88
<script>
99
import { NcModal } from '@nextcloud/vue'
10+
import { privateRoutes } from '../router'
1011
1112
export default {
1213
name: 'LoadingModal',
@@ -22,6 +23,7 @@ export default {
2223
moveSelection: this.t('bookmkarks', 'Moving selection'),
2324
copySelection: this.t('bookmkarks', 'Adding selection to folders'),
2425
emptyTrashbin: this.t('bookmkarks', 'Emptying trashbin'),
26+
deleted_folders: this.t('bookmkarks', 'Loading trashbin'),
2527
},
2628
showNcModal: false,
2729
showTimeout: null,
@@ -42,6 +44,9 @@ export default {
4244
},
4345
watch: {
4446
state(newState, previous) {
47+
if (this.state === 'deleted_folders' && this.$route.name !== privateRoutes.TRASHBIN) {
48+
return
49+
}
4550
if (this.state && !previous) {
4651
this.showTimeout = setTimeout(() => {
4752
this.showNcModal = true

src/store/actions.js

Lines changed: 17 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -799,6 +799,9 @@ export default {
799799
},
800800

801801
async [actions.LOAD_DELETED_FOLDERS]({ commit, dispatch, state }) {
802+
if (state.loading.deleted_folders) {
803+
return;
804+
}
802805
if (state.deletedFolders === null) {
803806
try {
804807
const count = loadState('bookmarks', 'deletedFolders')
@@ -826,11 +829,11 @@ export default {
826829
} = response
827830
if (status !== 'success') throw new Error(data)
828831
const folders = data
829-
commit(mutations.FETCH_END, 'folders')
832+
commit(mutations.FETCH_END, 'deleted_folders');
830833
return commit(mutations.SET_DELETED_FOLDERS, folders)
831834
} catch (err) {
832835
console.error(err)
833-
commit(mutations.FETCH_END, 'folders')
836+
commit(mutations.FETCH_END, 'deleted_folders');
834837
commit(
835838
mutations.SET_ERROR,
836839
AppGlobal.methods.t('bookmarks', 'Failed to load deleted folders'),
@@ -1656,35 +1659,22 @@ export default {
16561659
},
16571660
async [actions.EMPTY_TRASHBIN]({ commit, dispatch, state }) {
16581661
if (state.loading.emptyTrashbin) return
1662+
let canceled = false
16591663
await commit(mutations.FETCH_START, {
16601664
type: 'emptyTrashbin',
1665+
cancel() {
1666+
canceled = true
1667+
}
16611668
})
16621669
try {
1663-
await Parallel.each(
1664-
state.deletedFolders,
1665-
folder =>
1666-
dispatch(actions.DELETE_FOLDER, {
1667-
id: folder.id,
1668-
avoidReload: true,
1669-
hard: true,
1670-
}),
1671-
10,
1672-
)
1673-
await Parallel.each(
1674-
state.bookmarks,
1675-
(bookmark) => {
1676-
// soft delete all occurences instead of hard deleting the bookmark itself
1677-
return Promise.all(bookmark.folders.map((folder) => {
1678-
return dispatch(actions.DELETE_BOOKMARK, {
1679-
id: bookmark.id,
1680-
folder,
1681-
avoidReload: true,
1682-
hard: true,
1683-
})
1684-
}))
1685-
},
1686-
10,
1687-
)
1670+
const response = await axios.delete(url(state, '/folder/deleted'), {
1671+
params: {},
1672+
})
1673+
if (canceled) return
1674+
const {
1675+
data: { data, status },
1676+
} = response
1677+
if (status !== 'success') throw new Error(data)
16881678
dispatch(actions.RELOAD_VIEW)
16891679
dispatch(actions.LOAD_DELETED_FOLDERS)
16901680
commit(mutations.FETCH_END, 'emptyTrashbin')

0 commit comments

Comments
 (0)