Refactor dashboard to move widgets into modules. #5896

driusan · 2020-01-07T19:21:08Z

This updates the dashboard in order to move responsibility for dashboard widgets into the modules themselves. It adds the concept of a "widget" to LORIS as a way to provide loosely coupled cross-module dependencies. Modules may now register "widgets" of different types to be used by other modules. As a proof-of-concept, this architecture can be the basis of the upcoming candidates dashboard.

There are 2 types of widgets defined in this PR: dashboard widgets, which add a panel to the dashboard, and usertasks widgets, which add a task to the "My Tasks" panel on the dashboard.

All existing widgets are moved to the most appropriate module that currently exists. The dashboard only maintains ownership of the "Welcome" widget and the "My Tasks" widget itself.

driusan

Changes I intend to make before taking this out of draft mode, in case anyone's wondering.

modules/dashboard/php/widget.class.inc

driusan · 2020-01-07T19:28:25Z

modules/statistics/php/charts.class.inc

@@ -0,0 +1,343 @@
+<?php
+/**


Note: I'll probably move this endpoint into a different PR to simplify things.

src/GUI/TaskQueryWidget.php

src/GUI/TaskWidget.php

This adds a "renderTemplate" function to the module class, in order to render a template from a module, given its \Module descriptor. This is a helper extracted from aces#5896 which is mostly self-contained enough to stand on its own.

johnsaigle · 2020-01-07T19:53:15Z

It would be good to coordinate this with #5829

driusan · 2020-01-07T20:17:02Z

There is no way to coordinate with that PR other than blocking because git has no concept of code being moved from one place to another, it's just a bunch of lines deleted in one place and unrelated code added in another. As I said last year in a comment on that PR, it shouldn't be worked on until this is done.

(On the other hand, it should make it a lot easier to split up #5829 since PRs can be sent one widget or module at a time and there's really not much left in the dashboard itself to 'Reactify')

johnsaigle · 2020-01-07T20:18:28Z

OK it seems that you and @maltheism are in agreement that this PR will probably go forward first/instead of #5829. I just wanted to make sure everyone was clear on that.

This moves the query to get the last user login from the dashboard code to the User class, where it more accurately belongs. (This was extracted from aces#5896 where it was necessary because multiple modules use the last login time to determine if something is new.)

…ics module This moves the ajax calls from the dashboard module into the more appropriate statistics module for the graph endpoints. It also converts some inexplicable "post" requests into "get" requests but otherwise behaviour should remain unchanged. This was extracted from aces#5896 and is sent as a separate PR in order to simplify that PR and make it easier to rebase data/permission related PRs which would otherwise conflict.

driusan · 2020-01-07T21:55:33Z

Blocked primarily by #5839, but also by #5897, #5899, and #5900 (which are parts of this that were moved into separate PRs to make review easier, but are also technically already included in this branch since it's where they came from..)

The "Needs Work" is as described in my comments..

driusan · 2020-01-07T22:06:51Z

These bugs were also discovered/fixed fixed by this PR:

assigned issues did not show up in "My Tasks" even if there were issues assigned, unless someone had the developer permission (it now shows up or not based on if you have access to the issue tracker).
There were no multisite permissions for the violated scans or imaging browser QC "My Tasks" entries despite there being permission for that purpose in the database.

…ics module (#5900) This moves the ajax calls from the dashboard module into the more appropriate statistics module for the graph endpoints. It also converts some inexplicable "post" requests into "get" requests but otherwise behaviour should remain unchanged. This was extracted from #5896 and is sent as a separate PR in order to simplify that PR and make it easier to rebase data/permission related PRs which would otherwise conflict.

kongtiaowang · 2020-01-09T19:26:39Z

I will re-write the integration test and continue working on this PR.

driusan · 2020-01-09T19:34:18Z

The tests shouldn't need to be rewritten, the dashboard should behave identically to before with the default configuration

kongtiaowang · 2020-01-09T20:16:05Z

modules/bvl_feedback/php/module.class.inc

+                            [
+                                'baseURL'       => $baseURL,
+                                'notifications' => $frontend_feedback
+                            ],


remove " , "

Those shouldn't make a difference Travis only runs PHP 7.3+ but I'll remove them and see what happens.

kongtiaowang · 2020-01-09T20:17:52Z

modules/bvl_feedback/php/module.class.inc

+                                'notifications' => $frontend_feedback
+                            ],
+                        ),
+                        "",


I tried to run your PR locally, but I got all same error "syntax error, unexpected ')' in /var/www/Loris/modules/dashboard/php/module.class.inc on line 120"
modules/bvl_feedback/php/module.class.inc
modules/dashboard/php/dashboard.class.inc
modules/dashboard/php/module.class.inc
modules/document_repository/php/module.class.inc
modules/statistics/php/module.class.inc
You seem like add a unnecessary comma in the functioins.

What version of PHP are you using?

Trailing commas being optional in function calls was added to PHP in 7.3 (and Travis now only runs 7.3 and 7.4, so that's not why it's failing.. ), but I removed the comma anyways

kongtiaowang · 2020-01-10T16:27:09Z

modules/dashboard/test/DashboardTest.php

-                    "//*[@id='lorisworkspace']/div/di".
-                    "v[1]/div[2]/div[1]/div/div/button"
+                    "//*[@id='lorisworkspace']/div[1]".
+                    "/div[2]/div[1]/div/div/button"


Line 432-433 /html/body/div[1]/div/div/div/div/div[1]/div[2]/div[1]/div/div/ul/li[1]/a

line 439-440 /html/body/div[1]/div/div/div/div/div[1]/div[2]/div[1]/div/div/ul/li[2]/a

zaliqarosli · 2020-01-13T20:37:57Z

im getting the following error:
PHP Parse error: syntax error, unexpected ')' in /var/www/loris/modules/dashboard/php/module.class.inc on line 157

driusan · 2020-01-13T20:41:14Z

What version of PHP are you using? The minimum required version on the master branch is 7.3 and I'm not getting any syntax errors on that file with php -l

zaliqarosli · 2020-01-13T21:04:14Z

@driusan i also looked at the code and didn't see anything wrong. im on PHP 7.3.10

johnsaigle · 2020-01-13T21:11:56Z

The PR is working for me and there aren't any issues from make checkstatic.

driusan · 2020-01-13T21:14:49Z

@zaliqarosli are you sure your web server and command line are using the same version of PHP? It looks like the error is from php <= 7.2 because of a trailing comma in a function call's arguments to me.

zaliqarosli

looks good. there are some errors that pop up in my error log when testing but they are not due to this PR and already exist on master branch

christinerogers · 2020-05-04T15:15:33Z

Adding Needs Documentation label for the 23 release since the Readme needs updating, as @driusan's comment on the TestPlan update suggests
cc @ridz1208 because of the label -

driusan added State: Blocked PR or issue awaiting an external event such as the merge or another PR to proceed Cleanup PR or issue introducing/requiring at least one clean-up operation Meta PR does something that organizes, upgrades, or manages the functionality of the codebase labels Jan 7, 2020

driusan commented Jan 7, 2020

View reviewed changes

driusan mentioned this pull request Jan 7, 2020

[Core] Add renderTemplate helper to Module class #5897

Closed

driusan mentioned this pull request Jan 7, 2020

[Dashboard] module upgraded for react #5829

Closed

driusan mentioned this pull request Jan 7, 2020

[Core] Add getLastLogin helper to user class #5899

Merged

driusan mentioned this pull request Jan 7, 2020

[Dashboard/Statistics] Move dashboard ajax endpoints into the statistics module #5900

Merged

driusan added the State: Needs work PR awaiting additional work by the author to proceed label Jan 7, 2020

driusan added the Release: Add to release notes PR whose changes should be highlighted in the release notes label Jan 8, 2020

driusan force-pushed the RefactorDashboard branch from 2ffdaa8 to 4734533 Compare January 8, 2020 18:12

driusan added 5 commits January 8, 2020 14:59

WIP

51cba68

WIP

7b88c1d

Add bvl feedback widget

faaaefe

Update TODO

1013c38

Start adding My Tasks widget

57cdbf7

driusan added the "Help! I don't understand Travis!" PR is having a beef with TRAVIS. Someone needs to help label Jan 9, 2020

Fix SQL syntax

e34c2e1

kongtiaowang reviewed Jan 9, 2020

View reviewed changes

driusan added 4 commits January 9, 2020 15:23

Remove trailing commas

b0435c2

Remove another trailing comma

b90085a

Add class names to My Tasks items

502ee15

Exclude DCC from dashboard conflicts stats

060ac51

driusan force-pushed the RefactorDashboard branch from 91aeb9c to 060ac51 Compare January 10, 2020 16:02

Fix xpath for button, hopefully

ac1b02f

kongtiaowang reviewed Jan 10, 2020

View reviewed changes

More xpaths

4fb71a8

driusan removed the "Help! I don't understand Travis!" PR is having a beef with TRAVIS. Someone needs to help label Jan 10, 2020

racostas mentioned this pull request Jan 10, 2020

[Dashboard] Restricting users to see statistics only from sites they have access to. #5917

Closed

driusan requested review from kchatpar and zaliqarosli January 13, 2020 18:44

johnsaigle added the Passed manual tests PR has been successfully tested by at least one peer label Jan 13, 2020

zaliqarosli approved these changes Jan 13, 2020

View reviewed changes

driusan merged commit 2ba5882 into aces:master Jan 15, 2020

ridz1208 added this to the 23.0.0 milestone Jan 15, 2020

christinerogers added the State: Needs documentation PR that needs a more exhaustive documentation to proceed label May 4, 2020

christinerogers mentioned this pull request May 4, 2020

[Dashboard] Update Readme post-refactor for 23 release #6467

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor dashboard to move widgets into modules. #5896

Refactor dashboard to move widgets into modules. #5896

driusan commented Jan 7, 2020 •

edited

Loading

driusan left a comment

driusan Jan 7, 2020

johnsaigle commented Jan 7, 2020

driusan commented Jan 7, 2020

johnsaigle commented Jan 7, 2020

driusan commented Jan 7, 2020 •

edited

Loading

driusan commented Jan 7, 2020

kongtiaowang commented Jan 9, 2020

driusan commented Jan 9, 2020

kongtiaowang Jan 9, 2020

driusan Jan 9, 2020

kongtiaowang Jan 9, 2020

kongtiaowang Jan 9, 2020

driusan Jan 9, 2020

kongtiaowang Jan 9, 2020

driusan Jan 9, 2020

kongtiaowang Jan 10, 2020

zaliqarosli commented Jan 13, 2020

driusan commented Jan 13, 2020

zaliqarosli commented Jan 13, 2020

johnsaigle commented Jan 13, 2020

driusan commented Jan 13, 2020

zaliqarosli left a comment

christinerogers commented May 4, 2020

		@@ -0,0 +1,343 @@
		<?php
		/**

Refactor dashboard to move widgets into modules. #5896

Refactor dashboard to move widgets into modules. #5896

Conversation

driusan commented Jan 7, 2020 • edited Loading

driusan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnsaigle commented Jan 7, 2020

driusan commented Jan 7, 2020

johnsaigle commented Jan 7, 2020

driusan commented Jan 7, 2020 • edited Loading

driusan commented Jan 7, 2020

kongtiaowang commented Jan 9, 2020

driusan commented Jan 9, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zaliqarosli commented Jan 13, 2020

driusan commented Jan 13, 2020

zaliqarosli commented Jan 13, 2020

johnsaigle commented Jan 13, 2020

driusan commented Jan 13, 2020

zaliqarosli left a comment

Choose a reason for hiding this comment

christinerogers commented May 4, 2020

driusan commented Jan 7, 2020 •

edited

Loading

driusan commented Jan 7, 2020 •

edited

Loading