enh(dashboard): add recent pages dashboard widget #898

blizzz · 2023-09-21T19:23:06Z

Fixes: #113

The implementation is not necessarily the most performant one… but using the services of Collectives that already are at hand.

mejo- · 2023-09-25T12:00:32Z

Super nice, thanks @blizzz!

We probably should think about more performant solutions to fetch the list of recent pages for all collectives. What do you think about a dedicated getRecentPagesForCollectives(array $collectiveIds) function that gets the most recent pages for a list of collectives from the oc_filecache table along with a JOIN on oc_collectives_pages?

Something along the lines of:

SELECT p.*, fc.mtime FROM oc_filecache fc LEFT JOIN oc_collectives_pages p ON fc.fileId = p.file_id WHERE (fc.path LIKE 'appdata_123/collectives/13/%' OR fc.path LIKE 'appdata_123/collectives/14/%') AND mimetype=14;

mejo- · 2023-09-25T16:38:52Z

By the way, that's the corresponding issue that will be closed with this PR: #113

blizzz · 2023-09-29T09:56:14Z

@mejo- please see the latest commit that targets your suggestions and our conversation in talk.

P.S.: What is also fixed is a Readme title that appeared before, and Collective emojis are also added. Screenshot ^ is updated.

mejo-

Thanks a lot @blizzz, looks really good overall. I have some minor comments though.

lib/Service/RecentPagesService.php

lib/Model/RecentPage.php

lib/Dashboard/RecentPagesWidget.php

lib/Model/RecentPage.php

lib/Service/RecentPagesService.php

cypress · 2023-10-04T19:03:43Z

1 failed and 1 flaky tests on run #1242 ↗︎

1	252	0	0	1

Details:

enh(dashboard): add recent pages dashboard widget
Project: Collectives	Commit: `aa7febfe72`
Status: Failed	Duration: 15:00 💡
Started: Nov 7, 2023 7:43 PM	Ended: Nov 7, 2023 7:58 PM

cypress/e2e/page-list.spec.js • 1 failed test • Nextcloud stable25

View
Output

Test		Artifacts
Page list > Page trash > allows to trash and restore page with subpage and attachment		`Test Replay` `Screenshots`

cypress/e2e/page-list.spec.js • 1 flaky test • Nextcloud stable27

View
Output

Test		Artifacts
Page list > Page trash > allows to trash and restore page with subpage and attachment		`Test Replay` `Screenshots`

Review all test suite changes for PR #898 ↗︎

blizzz · 2023-11-02T21:14:23Z

not sure why the cypress tests are failing, this is rather a frontend thing. Similar failures reported at #936

lib/Service/CollectiveHelper.php

mejo-

I think a small Cypress smoke test that tests whether the recent pages widget displays and lists a page would be good. @blizzz would you like to look into this yourself?

Given that I'd like to release Collectives latest tomorrow we can either postpone this or I'll see whether I find time to do it myself later.

blizzz · 2023-11-07T09:26:30Z

I think a small Cypress smoke test that tests whether the recent pages widget displays and lists a page would be good. @blizzz would you like to look into this yourself?

Given that I'd like to release Collectives latest tomorrow we can either postpone this or I'll see whether I find time to do it myself later.

I cannot promise to accomplish this today.

juliusknorr · 2023-11-07T10:02:22Z

Let's follow up on that then I'd say

mejo-

Super nice, thanks again for the PR. Only two comments. I have both suggested code changes ready locally and could commit+push them to this PR to get the PR in quickly.

But also fine if you want to do the changes yourself.

mejo- · 2023-11-07T14:24:16Z

lib/Service/RecentPagesService.php

+			} else {
+				$title = basename($internalPath);
+			}
+			$url = $this->urlGenerator->linkToRoute('collectives.start.indexPath', ['path' => implode('/', $pathParts)]);


Suggested change

$url = $this->urlGenerator->linkToRoute('collectives.start.indexPath', ['path' => implode('/', $pathParts)]);

$fileIdSuffix = '?fileId=' . $row['file_id'];

$url = $this->urlGenerator->linkToRoute('collectives.start.indexPath', ['path' => implode('/', $pathParts)]) . $fileIdSuffix;

It would be nice to add the fileId query parameter to the URL, to make the URL more robust to title changes (e.g. when copying the URL somewhere else).

mejo- · 2023-11-07T15:08:27Z

lib/Service/RecentPagesService.php

+			if ($row['filename'] !== 'Readme.md') {
+				$pathParts[] = basename($row['filename'], PageInfo::SUFFIX);
+				$title = basename($row['filename'], PageInfo::SUFFIX);
+			} else {


Suggested change

} else {

} elseif ($internalPath === '' || $internalPath === '.') {

$title = $this->l10n->t('Landing page');

} else {

We have to special-treat landing pages here.

mejo- · 2023-11-07T15:30:00Z

I pushed a simple Cypress test.

Edit: the Cypress test depends on the fix for landing pages to succeed 🙈

Signed-off-by: Arthur Schiwon <[email protected]>

…tion - adds a simplified RecentPage model to avoid incomplete PageInfo instances - add RecentPageService with a method to fetch them for a specified user - adapts and simplifies RecenPagesWidget Signed-off-by: Arthur Schiwon <[email protected]>

- CollectiveHelper methods now return array with collective id as key - by catch: make use of fluid setter at RecentPage Signed-off-by: Arthur Schiwon <[email protected]>

Signed-off-by: Arthur Schiwon <[email protected]>

- psalm issue: vimeo/psalm#8258 Signed-off-by: Arthur Schiwon <[email protected]>

Signed-off-by: Arthur Schiwon <[email protected]>

… links Signed-off-by: Jonas <[email protected]>

Signed-off-by: Jonas <[email protected]>

blizzz requested a review from max-nextcloud September 21, 2023 19:23

blizzz force-pushed the feat/noad/dashboard-widget branch 2 times, most recently from 4a9d5bd to 85412d1 Compare September 22, 2023 09:52

blizzz marked this pull request as ready for review September 22, 2023 09:56

blizzz requested a review from mejo- September 22, 2023 09:56

blizzz added the 2. developing label Sep 22, 2023

blizzz force-pushed the feat/noad/dashboard-widget branch from 32006bb to 41618ee Compare September 29, 2023 10:05

mejo- requested changes Oct 2, 2023

View reviewed changes

blizzz force-pushed the feat/noad/dashboard-widget branch from 41618ee to f5a39f2 Compare October 4, 2023 18:49

blizzz force-pushed the feat/noad/dashboard-widget branch from f5a39f2 to 48109ce Compare October 4, 2023 19:11

juliusknorr force-pushed the feat/noad/dashboard-widget branch from 48109ce to c586066 Compare October 31, 2023 09:29

blizzz force-pushed the feat/noad/dashboard-widget branch 2 times, most recently from 01f1ba5 to 828e953 Compare November 2, 2023 20:58

juliusknorr reviewed Nov 2, 2023

View reviewed changes

lib/Service/CollectiveHelper.php Outdated Show resolved Hide resolved

blizzz force-pushed the feat/noad/dashboard-widget branch 2 times, most recently from 148a121 to f5aaa73 Compare November 3, 2023 11:26

blizzz requested a review from mejo- November 3, 2023 11:26

blizzz added 3. to review and removed 2. developing labels Nov 3, 2023

juliusknorr approved these changes Nov 3, 2023

View reviewed changes

mejo- reviewed Nov 7, 2023

View reviewed changes

mejo- requested changes Nov 7, 2023

View reviewed changes

mejo- force-pushed the feat/noad/dashboard-widget branch from f5aaa73 to abb2409 Compare November 7, 2023 15:29

mejo- force-pushed the feat/noad/dashboard-widget branch 2 times, most recently from 55dc145 to ca3c11b Compare November 7, 2023 16:16

blizzz and others added 10 commits November 7, 2023 20:34

enh(dashboard): add recent pages dashboard widget

42542c8

Signed-off-by: Arthur Schiwon <[email protected]>

fix(performance): reuse already fetched CollectiveInfo

47bb4ec

- CollectiveHelper methods now return array with collective id as key - by catch: make use of fluid setter at RecentPage Signed-off-by: Arthur Schiwon <[email protected]>

fix(php): restore PHP 7.4 compat

c010f27

Signed-off-by: Arthur Schiwon <[email protected]>

refactor(model): remove fallback avatar logic from model

02f6a52

Signed-off-by: Arthur Schiwon <[email protected]>

style(cleanup): remove dead code

b98e03d

Signed-off-by: Arthur Schiwon <[email protected]>

test(psalm): silence false positives

8324725

- psalm issue: vimeo/psalm#8258 Signed-off-by: Arthur Schiwon <[email protected]>

fix(frontend): revert return format of getCollectivesForUser

eb8dc30

Signed-off-by: Arthur Schiwon <[email protected]>

fix(RecentPagesService): Add title for landing page and add fileId to…

ac9098c

… links Signed-off-by: Jonas <[email protected]>

test(cypress): Add simple Cypress test for the dashboard widget

aa7febf

Signed-off-by: Jonas <[email protected]>

mejo- force-pushed the feat/noad/dashboard-widget branch from ca3c11b to aa7febf Compare November 7, 2023 19:34

mejo- approved these changes Nov 7, 2023

View reviewed changes

mejo- merged commit 8aacaf0 into main Nov 7, 2023
50 of 57 checks passed

delete-merged-branch bot deleted the feat/noad/dashboard-widget branch November 7, 2023 20:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

enh(dashboard): add recent pages dashboard widget #898

enh(dashboard): add recent pages dashboard widget #898

blizzz commented Sep 21, 2023 •

edited

Loading

mejo- commented Sep 25, 2023

mejo- commented Sep 25, 2023

blizzz commented Sep 29, 2023 •

edited

Loading

mejo- left a comment

cypress bot commented Oct 4, 2023 •

edited

Loading

blizzz commented Nov 2, 2023

mejo- left a comment

blizzz commented Nov 7, 2023

juliusknorr commented Nov 7, 2023

mejo- left a comment

mejo- Nov 7, 2023

mejo- Nov 7, 2023

mejo- commented Nov 7, 2023 •

edited

Loading

	$url = $this->urlGenerator->linkToRoute('collectives.start.indexPath', ['path' => implode('/', $pathParts)]);
	$fileIdSuffix = '?fileId=' . $row['file_id'];
	$url = $this->urlGenerator->linkToRoute('collectives.start.indexPath', ['path' => implode('/', $pathParts)]) . $fileIdSuffix;

enh(dashboard): add recent pages dashboard widget #898

enh(dashboard): add recent pages dashboard widget #898

Conversation

blizzz commented Sep 21, 2023 • edited Loading

mejo- commented Sep 25, 2023

mejo- commented Sep 25, 2023

blizzz commented Sep 29, 2023 • edited Loading

mejo- left a comment

Choose a reason for hiding this comment

cypress bot commented Oct 4, 2023 • edited Loading

1 failed and 1 flaky tests on run #1242 ↗︎

Review all test suite changes for PR #898 ↗︎

blizzz commented Nov 2, 2023

mejo- left a comment

Choose a reason for hiding this comment

blizzz commented Nov 7, 2023

juliusknorr commented Nov 7, 2023

mejo- left a comment

Choose a reason for hiding this comment

mejo- Nov 7, 2023

Choose a reason for hiding this comment

mejo- Nov 7, 2023

Choose a reason for hiding this comment

mejo- commented Nov 7, 2023 • edited Loading

blizzz commented Sep 21, 2023 •

edited

Loading

blizzz commented Sep 29, 2023 •

edited

Loading

cypress bot commented Oct 4, 2023 •

edited

Loading

mejo- commented Nov 7, 2023 •

edited

Loading