Details
- Reviewers
bchauvet anlambert - Group Reviewers
Reviewers - Maniphest Tasks
- T4071: add forge now - add columns to list of requests
- Commits
- rDWAPPSd9fb8522edc7: add_forge_now: Add new columns in requests moderation dashboard
Diff Detail
- Repository
- rDWAPPS Web applications
- Branch
- add-mod-name
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 28445 Build 44474: Phabricator diff pipeline on jenkins Jenkins console · Jenkins Build 44473: arc lint + arc unit
Event Timeline
Build is green
Patch application report for D7491 (id=27169)
Rebasing onto ad5add7d36...
Current branch diff-target is up to date.
Changes applied before test
commit 165edda1ef830bf960d4a2dfe4c7c3cd3edfb1e4 Author: anirudhlakhotia <sanjeev196945@gmail.com> Date: Mon Apr 4 00:52:03 2022 +0530 Added moderator name column commit 06669f588e0be7b4028f817eb4160eafee4916d6 Author: anirudhlakhotia <sanjeev196945@gmail.com> Date: Sun Apr 3 21:01:42 2022 +0530 Minor changes to diff commit d505a441e26070e637a28295ed303f3fbb6646ff Author: anirudhlakhotia <sanjeev196945@gmail.com> Date: Sun Apr 3 14:26:46 2022 +0530 added hyperlinks to URLs in Browse Requests tab
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1655/ for more details.
Build is green
Patch application report for D7491 (id=27180)
Could not rebase; Attempt merge onto 8ffd81760a...
Merge made by the 'recursive' strategy. CONTRIBUTORS | 1 + assets/src/bundles/add_forge/create-request.js | 12 +++++++++++- 2 files changed, 12 insertions(+), 1 deletion(-)
Changes applied before test
commit 7ed056d5986bd46cb9b4be45c731222d877685df Merge: 8ffd8176 7651ce4e Author: Jenkins user <jenkins@localhost> Date: Mon Apr 4 13:04:55 2022 +0000 Merge branch 'diff-target' into HEAD commit 7651ce4e64df04b1264788df96a05e9a8cb78413 Author: anirudhlakhotia <sanjeev196945@gmail.com> Date: Mon Apr 4 18:03:30 2022 +0530 Updated the Hyperlink feature with requested changes commit 1947b8f9569843fd34ae0fd3b123c957eb058f49 Author: anirudhlakhotia <sanjeev196945@gmail.com> Date: Sun Apr 3 14:26:46 2022 +0530 added hyperlinks to URLs in Browse Requests tab
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1662/ for more details.
Build is green
Patch application report for D7491 (id=27181)
Rebasing onto 8ffd81760a...
First, rewinding head to replay your work on top of it... Applying: added hyperlinks to URLs in Browse Requests tab Applying: Updated the Hyperlink feature with requested changes
Changes applied before test
commit 0f9626d97125a73084f78197000feb9c8add0ba7 Author: anirudhlakhotia <sanjeev196945@gmail.com> Date: Mon Apr 4 18:03:30 2022 +0530 Updated the Hyperlink feature with requested changes commit 9224e7e8c89bb8fc04d5e4b08f83ab52126c2663 Author: anirudhlakhotia <sanjeev196945@gmail.com> Date: Sun Apr 3 14:26:46 2022 +0530 added hyperlinks to URLs in Browse Requests tab
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1663/ for more details.
Build has FAILED
Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1713/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1713/console
Build has FAILED
Patch application report for D7491 (id=27429)
Rebasing onto 120e48bade...
Current branch diff-target is up to date.
Changes applied before test
commit a351e9b811587383f608ab9c974e67af4429a4d4 Author: anirudhlakhotia <sanjeev196945@gmail.com> Date: Mon Apr 4 00:52:03 2022 +0530 Added moderator name column
Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1714/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1714/console
Build is green
Patch application report for D7491 (id=27430)
Rebasing onto 120e48bade...
Current branch diff-target is up to date.
Changes applied before test
commit d4bf8ad68889330a18972cb5e63584eb9c766374 Author: anirudhlakhotia <sanjeev196945@gmail.com> Date: Mon Apr 4 00:52:03 2022 +0530 Added moderator name column
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1715/ for more details.
assets/src/bundles/add_forge/moderation-dashboard.js | ||
---|---|---|
55–60 | The forge contact name is not the moderator. The field needed here is the moderator who made the latest action on the request. Information should be found in the RequestHistory (actor w/ actor_role == "moderator") At the same time the last update time should be displayed (additional column) | |
swh/web/templates/add_forge_now/requests-moderation.html | ||
33 | add "last update date" column |
(Once applied the requested changes)
Also, if you could add a screenshot of the rendering result of your diff, that'd be neat [1]
[1] Edit the diff and drag'n drop the screenshot file into the text area and phabricator will do the rest.
Cheers,
Are these results of the current diff? Or once the requested changes have been applied?
@anirudhlakhotia, below are some hints about how to add the display of the last moderator name in the add forge requests admin dashboard:
- the purpose is to add a new field moderator_name in the data returned by the endpoint serving the data displayed by the databables
- in that code, page.object_list contains instances of AddForgeRequest django model (grep for it) to serialize and send for display with datatables
- to get the last moderator name, you need to get the list of AddForgeRequestHistory django models associated to each AddForgeRequest model to display (see that example to get the full history for a request) filtered by actor_role being MODERATOR
- as in our case we need to fetch history for multiple requests, you should be able to do that query in an efficient way using django queryset api
- once you get the moderators for each request, you need to insert a moderator_name field in the AddForgeRequest serialized data and display that new field in the datatables
You will need some tests data to work on, I suggest you to login as user (password: user) to create an add forge request from the Add Forge Create Web UI of your local dev version of swh-web.
Then logout and login as admin (password: admin) to act as a moderator for the request you created.
You need to click on the request id from the requests list admin view to enter its moderation dashboard.
Select "Waiting for feedback" in the dropdown, add a sample comment and click on submit, you will now have a moderator to display from the requests history.
Try to display that moderator name in first place, then we will see how to properly test this on the backend and client sides.
add_forge_now : Added Moderator Name and Last Modified Date to Admin Request Dashboard
Related to T4071
Build is green
Patch application report for D7491 (id=27882)
Rebasing onto 25bf6ca843...
Current branch diff-target is up to date.
Changes applied before test
commit 8438bcd9ee702f35af587217998f8bbad180011a Author: anirudhlakhotia <sanjeev196945@gmail.com> Date: Thu Apr 28 17:57:40 2022 +0530 Added Moderator Name and Last Modified Date Columns
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1757/ for more details.
swh/web/add_forge_now/views.py | ||
---|---|---|
77–81 ↗ | (On Diff #27882) | ? And this should be in the serializer |
From my point of view, there is still some improvements to bring to that diff and tests must be implemented to validate the feature.
I will do a deeper review once I have some time.
assets/src/bundles/add_forge/moderation-dashboard.js | ||
---|---|---|
51–62 | This change is not in the scope of that diff, it should be removed for this one and a new diff created for it. | |
swh/web/add_forge_now/views.py | ||
75–81 ↗ | (On Diff #27882) | You should not have to iterate on requests to filter history as you already got the history related to a request in line 72. Please use django queryset api instead by chaining filters to get AddForgeNowRequestHistory related to moderators. |
Updating D7491: Added Moderator Name and Last Modified Time columns to the Admin Request Dashboard
Build has FAILED
Patch application report for D7491 (id=27929)
Could not rebase; Attempt merge onto 25bf6ca843...
Updating 25bf6ca8..64d789bd Fast-forward .../src/bundles/add_forge/moderation-dashboard.js | 10 ++++++++ swh/web/add_forge_now/views.py | 3 +-- swh/web/api/views/add_forge_now.py | 29 ++++++++++++++++++++++ .../add_forge_now/requests-moderation.html | 2 ++ 4 files changed, 42 insertions(+), 2 deletions(-)
Changes applied before test
commit 64d789bd6af0a7aec608c5abe6c96ed05df09399 Author: anirudhlakhotia <sanjeev196945@gmail.com> Date: Mon May 2 16:31:44 2022 +0530 Added Moderator Name and Last Modified Date Columns commit 8438bcd9ee702f35af587217998f8bbad180011a Author: anirudhlakhotia <sanjeev196945@gmail.com> Date: Thu Apr 28 17:57:40 2022 +0530 Added Moderator Name and Last Modified Date Columns
Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1766/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1766/console
Updating D7491: Added Moderator Name and Last Modified Time columns to the Admin Request Dashboard
Build was aborted
Patch application report for D7491 (id=27939)
Could not rebase; Attempt merge onto 25bf6ca843...
Updating 25bf6ca8..d21096c6 Fast-forward .../src/bundles/add_forge/moderation-dashboard.js | 10 ++++++++ swh/web/add_forge_now/views.py | 3 +-- swh/web/api/views/add_forge_now.py | 29 ++++++++++++++++++++++ .../add_forge_now/requests-moderation.html | 2 ++ swh/web/tests/api/views/test_add_forge_now.py | 21 ++++++++++++---- 5 files changed, 58 insertions(+), 7 deletions(-)
Changes applied before test
commit d21096c6a8254faf60ecc8ee382a0370038153da Author: anirudhlakhotia <sanjeev196945@gmail.com> Date: Mon May 2 16:31:44 2022 +0530 Added Moderator Name and Last Modified Date Columns commit 8438bcd9ee702f35af587217998f8bbad180011a Author: anirudhlakhotia <sanjeev196945@gmail.com> Date: Thu Apr 28 17:57:40 2022 +0530 Added Moderator Name and Last Modified Date Columns
Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1767/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1767/console
Build was aborted
Patch application report for D7491 (id=27939)
Could not rebase; Attempt merge onto 25bf6ca843...
Updating 25bf6ca8..d21096c6 Fast-forward .../src/bundles/add_forge/moderation-dashboard.js | 10 ++++++++ swh/web/add_forge_now/views.py | 3 +-- swh/web/api/views/add_forge_now.py | 29 ++++++++++++++++++++++ .../add_forge_now/requests-moderation.html | 2 ++ swh/web/tests/api/views/test_add_forge_now.py | 21 ++++++++++++---- 5 files changed, 58 insertions(+), 7 deletions(-)
Changes applied before test
commit d21096c6a8254faf60ecc8ee382a0370038153da Author: anirudhlakhotia <sanjeev196945@gmail.com> Date: Mon May 2 16:31:44 2022 +0530 Added Moderator Name and Last Modified Date Columns commit 8438bcd9ee702f35af587217998f8bbad180011a Author: anirudhlakhotia <sanjeev196945@gmail.com> Date: Thu Apr 28 17:57:40 2022 +0530 Added Moderator Name and Last Modified Date Columns
Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1768/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1768/console
Updating D7491: Added Moderator Name and Last Modified Time columns to the Admin Request Dashboard
Build has FAILED
Patch application report for D7491 (id=27943)
Could not rebase; Attempt merge onto 4a6bc69b2f...
Updating 4a6bc69b..af2045ae Fast-forward .../src/bundles/add_forge/moderation-dashboard.js | 10 ++++++++ swh/web/add_forge_now/views.py | 3 +-- swh/web/api/views/add_forge_now.py | 29 ++++++++++++++++++++++ .../add_forge_now/requests-moderation.html | 2 ++ swh/web/tests/api/views/test_add_forge_now.py | 21 ++++++++++++---- 5 files changed, 58 insertions(+), 7 deletions(-)
Changes applied before test
commit af2045aefefe302f85fba5fac2461c07edf08cc6 Author: anirudhlakhotia <sanjeev196945@gmail.com> Date: Mon May 2 16:31:44 2022 +0530 Added Moderator Name and Last Modified Date Columns commit 035f704a79bf7053a36c641a018b8d0965c7a241 Author: anirudhlakhotia <sanjeev196945@gmail.com> Date: Thu Apr 28 17:57:40 2022 +0530 Added Moderator Name and Last Modified Date Columns
Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1770/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1770/console
Build has FAILED
Patch application report for D7491 (id=27943)
Could not rebase; Attempt merge onto 4a6bc69b2f...
Updating 4a6bc69b..af2045ae Fast-forward .../src/bundles/add_forge/moderation-dashboard.js | 10 ++++++++ swh/web/add_forge_now/views.py | 3 +-- swh/web/api/views/add_forge_now.py | 29 ++++++++++++++++++++++ .../add_forge_now/requests-moderation.html | 2 ++ swh/web/tests/api/views/test_add_forge_now.py | 21 ++++++++++++---- 5 files changed, 58 insertions(+), 7 deletions(-)
Changes applied before test
commit af2045aefefe302f85fba5fac2461c07edf08cc6 Author: anirudhlakhotia <sanjeev196945@gmail.com> Date: Mon May 2 16:31:44 2022 +0530 Added Moderator Name and Last Modified Date Columns commit 035f704a79bf7053a36c641a018b8d0965c7a241 Author: anirudhlakhotia <sanjeev196945@gmail.com> Date: Thu Apr 28 17:57:40 2022 +0530 Added Moderator Name and Last Modified Date Columns
Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1771/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1771/console
Build is green
Patch application report for D7491 (id=27943)
Could not rebase; Attempt merge onto 4a6bc69b2f...
Updating 4a6bc69b..af2045ae Fast-forward .../src/bundles/add_forge/moderation-dashboard.js | 10 ++++++++ swh/web/add_forge_now/views.py | 3 +-- swh/web/api/views/add_forge_now.py | 29 ++++++++++++++++++++++ .../add_forge_now/requests-moderation.html | 2 ++ swh/web/tests/api/views/test_add_forge_now.py | 21 ++++++++++++---- 5 files changed, 58 insertions(+), 7 deletions(-)
Changes applied before test
commit af2045aefefe302f85fba5fac2461c07edf08cc6 Author: anirudhlakhotia <sanjeev196945@gmail.com> Date: Mon May 2 16:31:44 2022 +0530 Added Moderator Name and Last Modified Date Columns commit 035f704a79bf7053a36c641a018b8d0965c7a241 Author: anirudhlakhotia <sanjeev196945@gmail.com> Date: Thu Apr 28 17:57:40 2022 +0530 Added Moderator Name and Last Modified Date Columns
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1772/ for more details.
Looks good to me. Before landing this, please squash the two comits into a single one and reword commit message the following way:
add_forge_now: Add new columns in requests moderation dashboard Add moderator name and last modified date columns. Closes T4071
add_forge_now: Add new columns in requests moderation dashboard
Add moderator name and last modified date columns.
Closes T4071
Build is green
Patch application report for D7491 (id=27954)
Rebasing onto 4a6bc69b2f...
Current branch diff-target is up to date.
Changes applied before test
commit a237d1af309095260a0786d093480a37467c953c Author: anirudhlakhotia <sanjeev196945@gmail.com> Date: Thu Apr 28 17:57:40 2022 +0530 add_forge_now: Add new columns in requests moderation dashboard Add moderator name and last modified date columns. Closes T4071
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1774/ for more details.
in the API and the internal code, you should call it "last_moderator" instead of "moderator". It's fine not to mention it in the UI, though.
add_forge_now: Add new columns in requests moderation dashboard
Add moderator name and last modified date columns.
Closes T4071
Build is green
Patch application report for D7491 (id=27956)
Rebasing onto 4a6bc69b2f...
Current branch diff-target is up to date.
Changes applied before test
commit d9fb8522edc75e60ab4649d49aabe1235c532916 Author: anirudhlakhotia <sanjeev196945@gmail.com> Date: Thu Apr 28 17:57:40 2022 +0530 add_forge_now: Add new columns in requests moderation dashboard Add moderator name and last modified date columns. Closes T4071
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1776/ for more details.