Details

Reviewers

vlorentz
anlambert

Group Reviewers

Reviewers

Maniphest Tasks

T3991: [Add forge now] Create moderation dashboard

Commits

rDWAPPS7e61894e9043: Add moderation view to list add-forge-now requests

Summary

In a future commit, it will be possible to browse from that view to the detailed request
dashboard.

This also reworks the left menu to allow access to staff or people with the add forge
now moderator permission.

Related to T3991
Depends on D7366

Test Plan

new cypress tests

Diff Detail

Repository

rDWAPPS Web applications

Branch

new-sprint-add-forge-now-moderation-view

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 27511
Build 43050: Phabricator diff pipeline on jenkins	Jenkins console · Jenkins
Build 43049: arc lint + arc unit

Unit TestsFailed
View All

	Time	Test
	423 ms	Jenkins > .tox.py3.lib.python3.7.site-packages.swh.web.tests.add_forge_now.test_views::Tests / Python tests / test_add_forge_request_list_datatables self = <django.db.backends.utils.CursorWrapper object at 0x7f0b6578c9e8> sql = 'INSERT INTO "django_content_type" ("app_label", "model") VALUES (%s, %s) RETURNING "django_content_type"."id"' params = ('swh.web.add_forge_now', 'dummy')
	351 ms	Jenkins > .tox.py3.lib.python3.7.site-packages.swh.web.tests.api.views.test_add_forge_now::Tests / Python tests / test_add_forge_request_get self = <django.db.backends.utils.CursorWrapper object at 0x7f0b65dfe048> sql = 'INSERT INTO "django_content_type" ("app_label", "model") VALUES (%s, %s) RETURNING "django_content_type"."id"' params = ('swh.web.add_forge_now', 'dummy')
	349 ms	Jenkins > .tox.py3.lib.python3.7.site-packages.swh.web.tests.api.views.test_add_forge_now::Tests / Python tests / test_add_forge_request_get_moderator self = <django.db.backends.utils.CursorWrapper object at 0x7f0b66bd4c18> sql = 'INSERT INTO "auth_permission" ("name", "content_type_id", "codename") VALUES (%s, %s, %s) RETURNING "auth_permission"."id"' params = ('moderator', 1, 'moderator')
	453 ms	Jenkins > .tox.py3.lib.python3.7.site-packages.swh.web.tests.api.views.test_add_forge_now::Tests / Python tests / test_add_forge_request_list_moderator self = <django.db.backends.utils.CursorWrapper object at 0x7f0b6a5c8a20> sql = 'INSERT INTO "django_content_type" ("app_label", "model") VALUES (%s, %s) RETURNING "django_content_type"."id"' params = ('swh.web.add_forge_now', 'dummy')
	349 ms	Jenkins > .tox.py3.lib.python3.7.site-packages.swh.web.tests.api.views.test_add_forge_now::Tests / Python tests / test_add_forge_request_update_empty self = <django.db.backends.utils.CursorWrapper object at 0x7f0b6678b470> sql = 'INSERT INTO "auth_permission" ("name", "content_type_id", "codename") VALUES (%s, %s, %s) RETURNING "auth_permission"."id"' params = ('moderator', 1, 'moderator')
		View Full Test Results (7 Failed · 904 Passed · 5 Skipped)

Event Timeline

ardumont created this revision.Mar 17 2022, 10:00 AM

Herald added a reviewer: Reviewers. · View Herald TranscriptMar 17 2022, 10:00 AM

ardumont mentioned this in D7348: wip: moderation: Start ui coverage and fix permissions.Mar 17 2022, 10:02 AM

swh-public-ci added a comment.Mar 17 2022, 10:02 AM

This comment was removed by ardumont.

Harbormaster returned this revision to the author for changes because remote builds failed.Mar 17 2022, 10:02 AM

Harbormaster failed remote builds in B27508: Diff 26625!

This comment was removed by ardumont.

It was a failed build for unrelated reasons. The commit log was too long so there dropped.

Rebase on top of D7366 (to separate the deposit changes from the moderation changes)

ardumont edited the summary of this revision. (Show Details)Mar 17 2022, 10:16 AM

ardumont added a parent revision: D7366: Fix access for people with permission access to deposit moderation.

ardumont removed a parent revision: D7357: Create form for "Add forge now".

Build has FAILED

Patch application report for D7365 (id=26627)

Could not rebase; Attempt merge onto 39bab96e9f...

Merge made by the 'recursive' strategy.
 assets/src/bundles/add_forge/create-request.js     |  88 ++++
 assets/src/bundles/add_forge/index.js              |  11 +
 .../src/bundles/add_forge/moderation-dashboard.js  |  50 +++
 cypress/fixtures/add-forge-now-requests.json       |  79 ++++
 .../integration/add-forge-now-moderation.spec.js   | 122 ++++++
 .../add-forge-now-request-create.spec.js           | 139 ++++++
 cypress/integration/deposit-admin.spec.js          |  52 ++-
 cypress/support/index.js                           |   4 +
 swh/web/add_forge_now/__init__.py                  |   0
 swh/web/add_forge_now/apps.py                      |  10 +
 swh/web/add_forge_now/migrations/0001_initial.py   | 109 +++++
 swh/web/add_forge_now/migrations/__init__.py       |   0
 swh/web/add_forge_now/models.py                    |  99 +++++
 swh/web/add_forge_now/tests/test_migration.py      |  62 +++
 swh/web/add_forge_now/views.py                     | 128 ++++++
 swh/web/api/urls.py                                |   1 +
 swh/web/api/views/add_forge_now.py                 | 355 +++++++++++++++
 swh/web/auth/utils.py                              |   1 +
 swh/web/common/utils.py                            |   6 +-
 swh/web/inbound_email/__init__.py                  |   0
 swh/web/inbound_email/apps.py                      |  11 +
 .../management/commands/process_inbound_email.py   |  73 ++++
 swh/web/inbound_email/signals.py                   |  36 ++
 swh/web/inbound_email/utils.py                     |  63 +++
 swh/web/settings/common.py                         |   2 +
 .../templates/add_forge_now/create-request.html    | 234 ++++++++++
 swh/web/templates/add_forge_now/moderation.html    |  45 ++
 swh/web/templates/layout.html                      |  18 +-
 swh/web/tests/add_forge_now/test_models.py         |  26 ++
 swh/web/tests/add_forge_now/test_views.py          | 203 +++++++++
 swh/web/tests/api/views/test_add_forge_now.py      | 483 +++++++++++++++++++++
 swh/web/tests/conftest.py                          |  12 +-
 swh/web/tests/create_test_admin.py                 |  10 +-
 swh/web/tests/create_test_users.py                 |  11 +-
 .../tests/inbound_email/test_management_command.py | 161 +++++++
 swh/web/tests/inbound_email/test_utils.py          | 113 +++++
 swh/web/tests/utils.py                             |   5 +-
 swh/web/urls.py                                    |   1 +
 38 files changed, 2809 insertions(+), 14 deletions(-)
 create mode 100644 assets/src/bundles/add_forge/create-request.js
 create mode 100644 assets/src/bundles/add_forge/index.js
 create mode 100644 assets/src/bundles/add_forge/moderation-dashboard.js
 create mode 100644 cypress/fixtures/add-forge-now-requests.json
 create mode 100644 cypress/integration/add-forge-now-moderation.spec.js
 create mode 100644 cypress/integration/add-forge-now-request-create.spec.js
 create mode 100644 swh/web/add_forge_now/__init__.py
 create mode 100644 swh/web/add_forge_now/apps.py
 create mode 100644 swh/web/add_forge_now/migrations/0001_initial.py
 create mode 100644 swh/web/add_forge_now/migrations/__init__.py
 create mode 100644 swh/web/add_forge_now/models.py
 create mode 100644 swh/web/add_forge_now/tests/test_migration.py
 create mode 100644 swh/web/add_forge_now/views.py
 create mode 100644 swh/web/api/views/add_forge_now.py
 create mode 100644 swh/web/inbound_email/__init__.py
 create mode 100644 swh/web/inbound_email/apps.py
 create mode 100644 swh/web/inbound_email/management/commands/process_inbound_email.py
 create mode 100644 swh/web/inbound_email/signals.py
 create mode 100644 swh/web/inbound_email/utils.py
 create mode 100644 swh/web/templates/add_forge_now/create-request.html
 create mode 100644 swh/web/templates/add_forge_now/moderation.html
 create mode 100644 swh/web/tests/add_forge_now/test_models.py
 create mode 100644 swh/web/tests/add_forge_now/test_views.py
 create mode 100644 swh/web/tests/api/views/test_add_forge_now.py
 create mode 100644 swh/web/tests/inbound_email/test_management_command.py
 create mode 100644 swh/web/tests/inbound_email/test_utils.py

Changes applied before test

commit cd323d12d627124119aecb277c9b3239682914e1
Merge: 39bab96e 92b1828f
Author: Jenkins user <jenkins@localhost>
Date:   Thu Mar 17 09:17:03 2022 +0000

    Merge branch 'diff-target' into HEAD

commit 92b1828fe92f3ec7c5a7d0995e5be992f7e4b423
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Mar 17 10:14:51 2022 +0100

    add-forge-now: Add moderation view
    
    This lists add-forge-now requests. In a future commit, it will be possible to browse to
    the detail dashboard request following a dedicated request.
    
    This reworks the left menu to allow access to staff or people with the add forge now
    moderator permission.
    
    Related to T3991

commit d9d554f425511f527a7f380ca0d06baec59d51b7
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Mar 17 10:13:22 2022 +0100

    Fix access for people with permission access to deposit moderation
    
    This will allow easier adaptations when adding new views with different permissions.

commit 654ffaf6ae69fdb1b1dec84388b5a97520c63cc6
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Wed Mar 16 18:09:46 2022 +0100

    Create "Add forge now" request
    
    Added a form to submit an "add forge now" request.
    Left menus changed to include "add forge now" page.
    Possbility to browse existing requests is added
    in a seperate tab in the same view.
    Cypress tests
    
    Related to T3998
    Related to T3989
    Related to T4012
    Related to T3988

commit 80e55b0a12eb27a5df72acf6fe07640db12eaf27
Merge: c24e6822 9f2e1cb9
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Wed Mar 16 16:21:35 2022 +0100

    Merge branch 'master' into sprint-add-forge-now

commit c24e6822a1569817b6d970fbc550cdf66f2188f0
Merge: ca39b06c 627ba79a
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Wed Mar 16 14:20:52 2022 +0100

    Merge branch 'master' into sprint-add-forge-now

commit ca39b06c46ba42f73f898377d3093299ea21a85a
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Wed Mar 9 16:37:54 2022 +0100

    inbound_email: add utilities to extract and filter email recipients

commit bf099ce9654d0b765f0820189857ab7c7beb6c6b
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Wed Mar 9 14:29:37 2022 +0100

    Bootstrap a common app for management of inbound emails

commit b6d540d2e5fafe04eb71d3b1d8f5c194dafcbb62
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Mar 15 15:06:10 2022 +0100

    add_forge_now: Move datatables listing in dedicated endpoint
    
    Merging datatables special processing in /api/1/add-forge/request/list
    endpoint implementation was quite disturbing.
    
    So better using a dedicated endpoint for that in a similar manner as
    with save code now requests listing.
    
    The endpoint to use with datatables is now the following one:
    /add-forge/request/list/datatables.
    
    Related to T3989
    Related to T3991

commit f6343395310bc226a50a283c88304006e8ece337
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Mon Mar 14 15:35:17 2022 +0100

    api/add_forge_now: Allow to use requests list endpoint with datatables
    
    datatables is the javascript library we use on the frontend side to
    display interactive tables.
    
    As we use server-side processing, table data must be provided in a
    paginated way by an HTTP endpoint.
    
    Response format expected by datatables is different from the one
    returned by the Web API endpoint listing add-forge requests.
    
    So adapt the response format of that endpoint when we know the
    input request has been sent by datatables.
    
    Related to T3989
    Related to T3991

commit 26748e56ecce5877fce3a215eba06a19ebb8342f
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Thu Mar 10 14:09:36 2022 +0100

    api: Add endpoint to get details about an add-forge request
    
    Related to T4030

commit 294a95c711bc40622c6d718afa2535f377b90dfc
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Mar 9 16:30:06 2022 +0100

    api: Add endpoint to list add-forge requests
    
    Related to T4027

commit 1bf17d6a75fd7265b1da9c9e87220efa62869c93
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Mar 9 16:28:03 2022 +0100

    api: Add endpoint to update an add-forge request
    
    Related to T4026

commit 130e9faa9bcab18bc3c76dc898546edb89f3f9b8
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Mar 8 15:23:50 2022 +0100

    api: Add endpoint to create an add-forge request
    
    Related to T3990

commit 03101208803501e5178f35d18d171e740ee4ca76
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Mar 8 11:34:44 2022 +0100

    add_forge_now: Bootstrap app and model

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1479/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1479/console

Harbormaster returned this revision to the author for changes because remote builds failed.Mar 17 2022, 10:18 AM

Harbormaster failed remote builds in B27510: Diff 26627!

Add missing chunk following commits separation

Build has FAILED

Patch application report for D7365 (id=26628)

Could not rebase; Attempt merge onto 39bab96e9f...

Merge made by the 'recursive' strategy.
 assets/src/bundles/add_forge/create-request.js     |  88 ++++
 assets/src/bundles/add_forge/index.js              |  11 +
 .../src/bundles/add_forge/moderation-dashboard.js  |  50 +++
 cypress/fixtures/add-forge-now-requests.json       |  79 ++++
 .../integration/add-forge-now-moderation.spec.js   | 122 ++++++
 .../add-forge-now-request-create.spec.js           | 139 ++++++
 cypress/integration/deposit-admin.spec.js          |  52 ++-
 cypress/support/index.js                           |   4 +
 swh/web/add_forge_now/__init__.py                  |   0
 swh/web/add_forge_now/apps.py                      |  10 +
 swh/web/add_forge_now/migrations/0001_initial.py   | 109 +++++
 swh/web/add_forge_now/migrations/__init__.py       |   0
 swh/web/add_forge_now/models.py                    |  99 +++++
 swh/web/add_forge_now/tests/test_migration.py      |  62 +++
 swh/web/add_forge_now/views.py                     | 128 ++++++
 swh/web/api/urls.py                                |   1 +
 swh/web/api/views/add_forge_now.py                 | 355 +++++++++++++++
 swh/web/auth/utils.py                              |   1 +
 swh/web/common/utils.py                            |   6 +-
 swh/web/inbound_email/__init__.py                  |   0
 swh/web/inbound_email/apps.py                      |  11 +
 .../management/commands/process_inbound_email.py   |  73 ++++
 swh/web/inbound_email/signals.py                   |  36 ++
 swh/web/inbound_email/utils.py                     |  63 +++
 swh/web/settings/common.py                         |   2 +
 .../templates/add_forge_now/create-request.html    | 234 ++++++++++
 swh/web/templates/add_forge_now/moderation.html    |  45 ++
 swh/web/templates/layout.html                      |  18 +-
 swh/web/tests/add_forge_now/test_models.py         |  26 ++
 swh/web/tests/add_forge_now/test_views.py          | 203 +++++++++
 swh/web/tests/api/views/test_add_forge_now.py      | 483 +++++++++++++++++++++
 swh/web/tests/conftest.py                          |  12 +-
 swh/web/tests/create_test_admin.py                 |  10 +-
 swh/web/tests/create_test_users.py                 |  20 +-
 .../tests/inbound_email/test_management_command.py | 161 +++++++
 swh/web/tests/inbound_email/test_utils.py          | 113 +++++
 swh/web/tests/utils.py                             |   5 +-
 swh/web/urls.py                                    |   1 +
 38 files changed, 2818 insertions(+), 14 deletions(-)
 create mode 100644 assets/src/bundles/add_forge/create-request.js
 create mode 100644 assets/src/bundles/add_forge/index.js
 create mode 100644 assets/src/bundles/add_forge/moderation-dashboard.js
 create mode 100644 cypress/fixtures/add-forge-now-requests.json
 create mode 100644 cypress/integration/add-forge-now-moderation.spec.js
 create mode 100644 cypress/integration/add-forge-now-request-create.spec.js
 create mode 100644 swh/web/add_forge_now/__init__.py
 create mode 100644 swh/web/add_forge_now/apps.py
 create mode 100644 swh/web/add_forge_now/migrations/0001_initial.py
 create mode 100644 swh/web/add_forge_now/migrations/__init__.py
 create mode 100644 swh/web/add_forge_now/models.py
 create mode 100644 swh/web/add_forge_now/tests/test_migration.py
 create mode 100644 swh/web/add_forge_now/views.py
 create mode 100644 swh/web/api/views/add_forge_now.py
 create mode 100644 swh/web/inbound_email/__init__.py
 create mode 100644 swh/web/inbound_email/apps.py
 create mode 100644 swh/web/inbound_email/management/commands/process_inbound_email.py
 create mode 100644 swh/web/inbound_email/signals.py
 create mode 100644 swh/web/inbound_email/utils.py
 create mode 100644 swh/web/templates/add_forge_now/create-request.html
 create mode 100644 swh/web/templates/add_forge_now/moderation.html
 create mode 100644 swh/web/tests/add_forge_now/test_models.py
 create mode 100644 swh/web/tests/add_forge_now/test_views.py
 create mode 100644 swh/web/tests/api/views/test_add_forge_now.py
 create mode 100644 swh/web/tests/inbound_email/test_management_command.py
 create mode 100644 swh/web/tests/inbound_email/test_utils.py

Changes applied before test

commit 92d9ae61e3ae2297446e6d5930ccc9b147ec1eac
Merge: 39bab96e c8b9d750
Author: Jenkins user <jenkins@localhost>
Date:   Thu Mar 17 09:21:52 2022 +0000

    Merge branch 'diff-target' into HEAD

commit c8b9d7501a6f131c51903c2b56c6bcf8a00a2e03
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Mar 17 10:14:51 2022 +0100

    add-forge-now: Add moderation view
    
    This lists add-forge-now requests. In a future commit, it will be possible to browse to
    the detail dashboard request following a dedicated request.
    
    This reworks the left menu to allow access to staff or people with the add forge now
    moderator permission.
    
    Related to T3991

commit d9d554f425511f527a7f380ca0d06baec59d51b7
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Mar 17 10:13:22 2022 +0100

    Fix access for people with permission access to deposit moderation
    
    This will allow easier adaptations when adding new views with different permissions.

commit 654ffaf6ae69fdb1b1dec84388b5a97520c63cc6
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Wed Mar 16 18:09:46 2022 +0100

    Create "Add forge now" request
    
    Added a form to submit an "add forge now" request.
    Left menus changed to include "add forge now" page.
    Possbility to browse existing requests is added
    in a seperate tab in the same view.
    Cypress tests
    
    Related to T3998
    Related to T3989
    Related to T4012
    Related to T3988

commit 80e55b0a12eb27a5df72acf6fe07640db12eaf27
Merge: c24e6822 9f2e1cb9
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Wed Mar 16 16:21:35 2022 +0100

    Merge branch 'master' into sprint-add-forge-now

commit c24e6822a1569817b6d970fbc550cdf66f2188f0
Merge: ca39b06c 627ba79a
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Wed Mar 16 14:20:52 2022 +0100

    Merge branch 'master' into sprint-add-forge-now

commit ca39b06c46ba42f73f898377d3093299ea21a85a
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Wed Mar 9 16:37:54 2022 +0100

    inbound_email: add utilities to extract and filter email recipients

commit bf099ce9654d0b765f0820189857ab7c7beb6c6b
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Wed Mar 9 14:29:37 2022 +0100

    Bootstrap a common app for management of inbound emails

commit b6d540d2e5fafe04eb71d3b1d8f5c194dafcbb62
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Mar 15 15:06:10 2022 +0100

    add_forge_now: Move datatables listing in dedicated endpoint
    
    Merging datatables special processing in /api/1/add-forge/request/list
    endpoint implementation was quite disturbing.
    
    So better using a dedicated endpoint for that in a similar manner as
    with save code now requests listing.
    
    The endpoint to use with datatables is now the following one:
    /add-forge/request/list/datatables.
    
    Related to T3989
    Related to T3991

commit f6343395310bc226a50a283c88304006e8ece337
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Mon Mar 14 15:35:17 2022 +0100

    api/add_forge_now: Allow to use requests list endpoint with datatables
    
    datatables is the javascript library we use on the frontend side to
    display interactive tables.
    
    As we use server-side processing, table data must be provided in a
    paginated way by an HTTP endpoint.
    
    Response format expected by datatables is different from the one
    returned by the Web API endpoint listing add-forge requests.
    
    So adapt the response format of that endpoint when we know the
    input request has been sent by datatables.
    
    Related to T3989
    Related to T3991

commit 26748e56ecce5877fce3a215eba06a19ebb8342f
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Thu Mar 10 14:09:36 2022 +0100

    api: Add endpoint to get details about an add-forge request
    
    Related to T4030

commit 294a95c711bc40622c6d718afa2535f377b90dfc
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Mar 9 16:30:06 2022 +0100

    api: Add endpoint to list add-forge requests
    
    Related to T4027

commit 1bf17d6a75fd7265b1da9c9e87220efa62869c93
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Mar 9 16:28:03 2022 +0100

    api: Add endpoint to update an add-forge request
    
    Related to T4026

commit 130e9faa9bcab18bc3c76dc898546edb89f3f9b8
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Mar 8 15:23:50 2022 +0100

    api: Add endpoint to create an add-forge request
    
    Related to T3990

commit 03101208803501e5178f35d18d171e740ee4ca76
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Mar 8 11:34:44 2022 +0100

    add_forge_now: Bootstrap app and model

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1481/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1481/console

Harbormaster returned this revision to the author for changes because remote builds failed.Mar 17 2022, 10:39 AM

Harbormaster failed remote builds in B27511: Diff 26628!

ardumont mentioned this in D7366: Fix access for people with permission access to deposit moderation.Mar 17 2022, 11:29 AM

Rebase

anlambert added a subscriber: anlambert.Mar 17 2022, 11:49 AM

anlambert added inline comments.

swh/web/add_forge_now/views.py
103–116	You should create a new `add_forge_now.py` file in the `admin` folder instead and put that code there. Also the URL of the view should be `/admin/add-forge/requests` instead of `/admin/moderation`.
swh/web/tests/create_test_users.py
27	I would rather name that user `add-forge-moderator`.

anlambert added inline comments.Mar 17 2022, 11:52 AM

swh/web/add_forge_now/views.py
114	You can remove the `AddForgeRequest.objects.all()` request now that data are fetched by datatables.

Build has FAILED

Patch application report for D7365 (id=26631)

Could not rebase; Attempt merge onto 39bab96e9f...

Merge made by the 'recursive' strategy.
 assets/src/bundles/add_forge/create-request.js     |  92 ++++
 assets/src/bundles/add_forge/index.js              |  11 +
 .../src/bundles/add_forge/moderation-dashboard.js  |  50 +++
 cypress/fixtures/add-forge-now-requests.json       |  79 ++++
 .../integration/add-forge-now-moderation.spec.js   | 122 ++++++
 .../add-forge-now-request-create.spec.js           | 139 ++++++
 cypress/integration/deposit-admin.spec.js          |  52 ++-
 cypress/support/index.js                           |   8 +
 swh/web/add_forge_now/__init__.py                  |   0
 swh/web/add_forge_now/apps.py                      |  10 +
 swh/web/add_forge_now/migrations/0001_initial.py   | 109 +++++
 swh/web/add_forge_now/migrations/__init__.py       |   0
 swh/web/add_forge_now/models.py                    |  99 +++++
 swh/web/add_forge_now/tests/test_migration.py      |  62 +++
 swh/web/add_forge_now/views.py                     | 128 ++++++
 swh/web/api/urls.py                                |   1 +
 swh/web/api/views/add_forge_now.py                 | 355 +++++++++++++++
 swh/web/auth/utils.py                              |   1 +
 swh/web/common/utils.py                            |   6 +-
 swh/web/inbound_email/__init__.py                  |   0
 swh/web/inbound_email/apps.py                      |  11 +
 .../management/commands/process_inbound_email.py   |  73 ++++
 swh/web/inbound_email/signals.py                   |  36 ++
 swh/web/inbound_email/utils.py                     |  63 +++
 swh/web/settings/common.py                         |   2 +
 .../templates/add_forge_now/create-request.html    | 234 ++++++++++
 swh/web/templates/add_forge_now/moderation.html    |  45 ++
 swh/web/templates/layout.html                      |  18 +-
 swh/web/tests/add_forge_now/test_models.py         |  26 ++
 swh/web/tests/add_forge_now/test_views.py          | 203 +++++++++
 swh/web/tests/api/views/test_add_forge_now.py      | 483 +++++++++++++++++++++
 swh/web/tests/conftest.py                          |  12 +-
 swh/web/tests/create_test_admin.py                 |  10 +-
 swh/web/tests/create_test_users.py                 |  21 +-
 .../tests/inbound_email/test_management_command.py | 161 +++++++
 swh/web/tests/inbound_email/test_utils.py          | 113 +++++
 swh/web/tests/utils.py                             |  14 +-
 swh/web/urls.py                                    |   1 +
 38 files changed, 2834 insertions(+), 16 deletions(-)
 create mode 100644 assets/src/bundles/add_forge/create-request.js
 create mode 100644 assets/src/bundles/add_forge/index.js
 create mode 100644 assets/src/bundles/add_forge/moderation-dashboard.js
 create mode 100644 cypress/fixtures/add-forge-now-requests.json
 create mode 100644 cypress/integration/add-forge-now-moderation.spec.js
 create mode 100644 cypress/integration/add-forge-now-request-create.spec.js
 create mode 100644 swh/web/add_forge_now/__init__.py
 create mode 100644 swh/web/add_forge_now/apps.py
 create mode 100644 swh/web/add_forge_now/migrations/0001_initial.py
 create mode 100644 swh/web/add_forge_now/migrations/__init__.py
 create mode 100644 swh/web/add_forge_now/models.py
 create mode 100644 swh/web/add_forge_now/tests/test_migration.py
 create mode 100644 swh/web/add_forge_now/views.py
 create mode 100644 swh/web/api/views/add_forge_now.py
 create mode 100644 swh/web/inbound_email/__init__.py
 create mode 100644 swh/web/inbound_email/apps.py
 create mode 100644 swh/web/inbound_email/management/commands/process_inbound_email.py
 create mode 100644 swh/web/inbound_email/signals.py
 create mode 100644 swh/web/inbound_email/utils.py
 create mode 100644 swh/web/templates/add_forge_now/create-request.html
 create mode 100644 swh/web/templates/add_forge_now/moderation.html
 create mode 100644 swh/web/tests/add_forge_now/test_models.py
 create mode 100644 swh/web/tests/add_forge_now/test_views.py
 create mode 100644 swh/web/tests/api/views/test_add_forge_now.py
 create mode 100644 swh/web/tests/inbound_email/test_management_command.py
 create mode 100644 swh/web/tests/inbound_email/test_utils.py

Changes applied before test

commit e0379c385a152b1f7d40e829aeca15f3b50b9ed6
Merge: 39bab96e 073d6684
Author: Jenkins user <jenkins@localhost>
Date:   Thu Mar 17 10:48:32 2022 +0000

    Merge branch 'diff-target' into HEAD

commit 073d6684132c0fa24e65331fe1c16fc2cf540fff
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Mar 17 10:14:51 2022 +0100

    add-forge-now: Add moderation view
    
    This lists add-forge-now requests. In a future commit, it will be possible to browse to
    the detail dashboard request following a dedicated request.
    
    This reworks the left menu to allow access to staff or people with the add forge now
    moderator permission.
    
    Related to T3991

commit ab461ecc4e8351014dabbb3b5e39a2632d9e2dd1
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Mar 17 10:13:22 2022 +0100

    Fix access for people with permission access to deposit moderation
    
    This will allow easier adaptations when adding new views with different permissions.

commit 6c96f964adc7ffb7e9ea32de6d240a6a45bd8b97
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Thu Mar 17 11:33:57 2022 +0100

    Create "Add forge now" request
    
    Added a form to submit an "add forge now" request.
    Left menus changed to include "add forge now" page.
    Possbility to browse existing requests is added
    in a seperate tab in the same view.
    Cypress tests
    
    Related to T3998
    Related to T3989
    Related to T4012
    Related to T3988

commit 80e55b0a12eb27a5df72acf6fe07640db12eaf27
Merge: c24e6822 9f2e1cb9
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Wed Mar 16 16:21:35 2022 +0100

    Merge branch 'master' into sprint-add-forge-now

commit c24e6822a1569817b6d970fbc550cdf66f2188f0
Merge: ca39b06c 627ba79a
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Wed Mar 16 14:20:52 2022 +0100

    Merge branch 'master' into sprint-add-forge-now

commit ca39b06c46ba42f73f898377d3093299ea21a85a
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Wed Mar 9 16:37:54 2022 +0100

    inbound_email: add utilities to extract and filter email recipients

commit bf099ce9654d0b765f0820189857ab7c7beb6c6b
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Wed Mar 9 14:29:37 2022 +0100

    Bootstrap a common app for management of inbound emails

commit b6d540d2e5fafe04eb71d3b1d8f5c194dafcbb62
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Mar 15 15:06:10 2022 +0100

    add_forge_now: Move datatables listing in dedicated endpoint
    
    Merging datatables special processing in /api/1/add-forge/request/list
    endpoint implementation was quite disturbing.
    
    So better using a dedicated endpoint for that in a similar manner as
    with save code now requests listing.
    
    The endpoint to use with datatables is now the following one:
    /add-forge/request/list/datatables.
    
    Related to T3989
    Related to T3991

commit f6343395310bc226a50a283c88304006e8ece337
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Mon Mar 14 15:35:17 2022 +0100

    api/add_forge_now: Allow to use requests list endpoint with datatables
    
    datatables is the javascript library we use on the frontend side to
    display interactive tables.
    
    As we use server-side processing, table data must be provided in a
    paginated way by an HTTP endpoint.
    
    Response format expected by datatables is different from the one
    returned by the Web API endpoint listing add-forge requests.
    
    So adapt the response format of that endpoint when we know the
    input request has been sent by datatables.
    
    Related to T3989
    Related to T3991

commit 26748e56ecce5877fce3a215eba06a19ebb8342f
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Thu Mar 10 14:09:36 2022 +0100

    api: Add endpoint to get details about an add-forge request
    
    Related to T4030

commit 294a95c711bc40622c6d718afa2535f377b90dfc
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Mar 9 16:30:06 2022 +0100

    api: Add endpoint to list add-forge requests
    
    Related to T4027

commit 1bf17d6a75fd7265b1da9c9e87220efa62869c93
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Mar 9 16:28:03 2022 +0100

    api: Add endpoint to update an add-forge request
    
    Related to T4026

commit 130e9faa9bcab18bc3c76dc898546edb89f3f9b8
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Mar 8 15:23:50 2022 +0100

    api: Add endpoint to create an add-forge request
    
    Related to T3990

commit 03101208803501e5178f35d18d171e740ee4ca76
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Mar 8 11:34:44 2022 +0100

    add_forge_now: Bootstrap app and model

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1484/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1484/console

Harbormaster returned this revision to the author for changes because remote builds failed.Mar 17 2022, 12:07 PM

Harbormaster failed remote builds in B27514: Diff 26631!

ardumont marked an inline comment as done.Mar 17 2022, 2:34 PM

ardumont added inline comments.

swh/web/add_forge_now/views.py
103–116	You should create a new add_forge_now.py file in the admin folder instead and put that code there. ok but why did we even bother creating a new django app? I mean if we end up adding every code inside existing arborescence tree.
114	oopsy, it got forgotten during refactoring.

ardumont added inline comments.Mar 17 2022, 2:53 PM

swh/web/add_forge_now/views.py
114	Also the URL of the view should be /admin/add-forge/requests instead of /admin/moderation. ok but why?

ardumont marked 2 inline comments as done.Mar 17 2022, 2:53 PM

Adapt according to review

Test failing in build does not fail locally...

ardumont added inline comments.Mar 17 2022, 3:21 PM

swh/web/admin/add_forge_now.py
27 ↗	(On Diff #26647)	i still need to drop that (locally done), waiting patiently for the build to finish first.

Build is green

Patch application report for D7365 (id=26647)

Could not rebase; Attempt merge onto 7bfbda85ff...

Merge made by the 'recursive' strategy.
 assets/src/bundles/add_forge/create-request.js     |  92 ++++
 assets/src/bundles/add_forge/index.js              |  11 +
 .../src/bundles/add_forge/moderation-dashboard.js  |  50 +++
 cypress/fixtures/add-forge-now-requests.json       |  79 ++++
 .../add-forge-now-request-create.spec.js           | 139 ++++++
 .../add-forge-now-requests-moderation.spec.js      | 122 ++++++
 cypress/integration/deposit-admin.spec.js          |  52 ++-
 cypress/support/index.js                           |   8 +
 swh/web/add_forge_now/__init__.py                  |   0
 swh/web/add_forge_now/apps.py                      |  10 +
 swh/web/add_forge_now/migrations/0001_initial.py   | 109 +++++
 swh/web/add_forge_now/migrations/__init__.py       |   0
 swh/web/add_forge_now/models.py                    |  99 +++++
 swh/web/add_forge_now/tests/test_migration.py      |  62 +++
 swh/web/add_forge_now/views.py                     | 107 +++++
 swh/web/admin/add_forge_now.py                     |  30 ++
 swh/web/admin/urls.py                              |   3 +-
 swh/web/api/urls.py                                |   1 +
 swh/web/api/views/add_forge_now.py                 | 355 +++++++++++++++
 swh/web/auth/utils.py                              |   1 +
 swh/web/common/utils.py                            |   6 +-
 swh/web/inbound_email/__init__.py                  |   0
 swh/web/inbound_email/apps.py                      |  11 +
 .../management/commands/process_inbound_email.py   |  73 ++++
 swh/web/inbound_email/signals.py                   |  36 ++
 swh/web/inbound_email/utils.py                     |  63 +++
 swh/web/settings/common.py                         |   2 +
 .../templates/add_forge_now/create-request.html    | 234 ++++++++++
 .../add_forge_now/requests-moderation.html         |  45 ++
 swh/web/templates/layout.html                      |  18 +-
 swh/web/tests/add_forge_now/test_models.py         |  26 ++
 swh/web/tests/add_forge_now/test_views.py          | 203 +++++++++
 swh/web/tests/api/views/test_add_forge_now.py      | 483 +++++++++++++++++++++
 swh/web/tests/conftest.py                          |  12 +-
 swh/web/tests/create_test_admin.py                 |  10 +-
 swh/web/tests/create_test_users.py                 |  23 +-
 .../tests/inbound_email/test_management_command.py | 161 +++++++
 swh/web/tests/inbound_email/test_utils.py          | 113 +++++
 swh/web/tests/utils.py                             |  14 +-
 swh/web/urls.py                                    |   1 +
 40 files changed, 2846 insertions(+), 18 deletions(-)
 create mode 100644 assets/src/bundles/add_forge/create-request.js
 create mode 100644 assets/src/bundles/add_forge/index.js
 create mode 100644 assets/src/bundles/add_forge/moderation-dashboard.js
 create mode 100644 cypress/fixtures/add-forge-now-requests.json
 create mode 100644 cypress/integration/add-forge-now-request-create.spec.js
 create mode 100644 cypress/integration/add-forge-now-requests-moderation.spec.js
 create mode 100644 swh/web/add_forge_now/__init__.py
 create mode 100644 swh/web/add_forge_now/apps.py
 create mode 100644 swh/web/add_forge_now/migrations/0001_initial.py
 create mode 100644 swh/web/add_forge_now/migrations/__init__.py
 create mode 100644 swh/web/add_forge_now/models.py
 create mode 100644 swh/web/add_forge_now/tests/test_migration.py
 create mode 100644 swh/web/add_forge_now/views.py
 create mode 100644 swh/web/admin/add_forge_now.py
 create mode 100644 swh/web/api/views/add_forge_now.py
 create mode 100644 swh/web/inbound_email/__init__.py
 create mode 100644 swh/web/inbound_email/apps.py
 create mode 100644 swh/web/inbound_email/management/commands/process_inbound_email.py
 create mode 100644 swh/web/inbound_email/signals.py
 create mode 100644 swh/web/inbound_email/utils.py
 create mode 100644 swh/web/templates/add_forge_now/create-request.html
 create mode 100644 swh/web/templates/add_forge_now/requests-moderation.html
 create mode 100644 swh/web/tests/add_forge_now/test_models.py
 create mode 100644 swh/web/tests/add_forge_now/test_views.py
 create mode 100644 swh/web/tests/api/views/test_add_forge_now.py
 create mode 100644 swh/web/tests/inbound_email/test_management_command.py
 create mode 100644 swh/web/tests/inbound_email/test_utils.py

Changes applied before test

commit 9458096b6f280eaf2c2564c459e258ac5769857f
Merge: 7bfbda85 37ac5300
Author: Jenkins user <jenkins@localhost>
Date:   Thu Mar 17 14:10:35 2022 +0000

    Merge branch 'diff-target' into HEAD

commit 37ac5300685dee50ee0f7bfd7cf761b634c7ccc6
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Mar 17 10:14:51 2022 +0100

    add-forge-now: Add moderation view
    
    This lists add-forge-now requests. In a future commit, it will be possible to browse to
    the detail dashboard request following a dedicated request.
    
    This reworks the left menu to allow access to staff or people with the add forge now
    moderator permission.
    
    Related to T3991

commit 4e23aa2eb8fca1f5c62a745f44f2f4b3b46003a9
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Mar 17 10:13:22 2022 +0100

    Fix access for people with permission access to deposit moderation
    
    This will allow easier adaptations when adding new views with different permissions.

commit 6c96f964adc7ffb7e9ea32de6d240a6a45bd8b97
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Thu Mar 17 11:33:57 2022 +0100

    Create "Add forge now" request
    
    Added a form to submit an "add forge now" request.
    Left menus changed to include "add forge now" page.
    Possbility to browse existing requests is added
    in a seperate tab in the same view.
    Cypress tests
    
    Related to T3998
    Related to T3989
    Related to T4012
    Related to T3988

commit 80e55b0a12eb27a5df72acf6fe07640db12eaf27
Merge: c24e6822 9f2e1cb9
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Wed Mar 16 16:21:35 2022 +0100

    Merge branch 'master' into sprint-add-forge-now

commit c24e6822a1569817b6d970fbc550cdf66f2188f0
Merge: ca39b06c 627ba79a
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Wed Mar 16 14:20:52 2022 +0100

    Merge branch 'master' into sprint-add-forge-now

commit ca39b06c46ba42f73f898377d3093299ea21a85a
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Wed Mar 9 16:37:54 2022 +0100

    inbound_email: add utilities to extract and filter email recipients

commit bf099ce9654d0b765f0820189857ab7c7beb6c6b
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Wed Mar 9 14:29:37 2022 +0100

    Bootstrap a common app for management of inbound emails

commit b6d540d2e5fafe04eb71d3b1d8f5c194dafcbb62
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Mar 15 15:06:10 2022 +0100

    add_forge_now: Move datatables listing in dedicated endpoint
    
    Merging datatables special processing in /api/1/add-forge/request/list
    endpoint implementation was quite disturbing.
    
    So better using a dedicated endpoint for that in a similar manner as
    with save code now requests listing.
    
    The endpoint to use with datatables is now the following one:
    /add-forge/request/list/datatables.
    
    Related to T3989
    Related to T3991

commit f6343395310bc226a50a283c88304006e8ece337
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Mon Mar 14 15:35:17 2022 +0100

    api/add_forge_now: Allow to use requests list endpoint with datatables
    
    datatables is the javascript library we use on the frontend side to
    display interactive tables.
    
    As we use server-side processing, table data must be provided in a
    paginated way by an HTTP endpoint.
    
    Response format expected by datatables is different from the one
    returned by the Web API endpoint listing add-forge requests.
    
    So adapt the response format of that endpoint when we know the
    input request has been sent by datatables.
    
    Related to T3989
    Related to T3991

commit 26748e56ecce5877fce3a215eba06a19ebb8342f
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Thu Mar 10 14:09:36 2022 +0100

    api: Add endpoint to get details about an add-forge request
    
    Related to T4030

commit 294a95c711bc40622c6d718afa2535f377b90dfc
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Mar 9 16:30:06 2022 +0100

    api: Add endpoint to list add-forge requests
    
    Related to T4027

commit 1bf17d6a75fd7265b1da9c9e87220efa62869c93
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Mar 9 16:28:03 2022 +0100

    api: Add endpoint to update an add-forge request
    
    Related to T4026

commit 130e9faa9bcab18bc3c76dc898546edb89f3f9b8
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Mar 8 15:23:50 2022 +0100

    api: Add endpoint to create an add-forge request
    
    Related to T3990

commit 03101208803501e5178f35d18d171e740ee4ca76
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Mar 8 11:34:44 2022 +0100

    add_forge_now: Bootstrap app and model

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1487/ for more details.

Harbormaster completed remote builds in B27530: Diff 26647.Mar 17 2022, 3:24 PM

ardumont requested review of this revision.Mar 17 2022, 3:24 PM

Please rename the commit and diff to make it clear you're adding a list of requests; currently it's not clear which moderation view you are adding.

And you need to sanitize forge URLs before display; currently nothing is stopping me from submitting <script>alert("foo");</script> as forge URL, and it gets interpreted when Datatable inserts it in the DOM. (or as a less malicious example, http://example.org/&=foo is displayed as http://example.org/&=foo

swh/web/templates/add_forge_now/requests-moderation.html
17 ↗	(On Diff #26647)	`{{ heading }}` is empty
37 ↗	(On Diff #26647)	please show something in this div, in case `wh.add_forge.onModerationPageLoad()` for some reason.
swh/web/templates/layout.html
237	For consistency with Save Code Now + don't make the sidebar larger

This revision now requires changes to proceed.Mar 17 2022, 3:34 PM

anlambert added inline comments.Mar 17 2022, 3:35 PM

assets/src/bundles/add_forge/moderation-dashboard.js
20	you can activate search feature, it is implemented in backend endpoint
swh/web/templates/layout.html
237	I think you can drop the moderation word in the menu entry text: this is shorter and we already know that it is an administration link.

And you need to sanitize forge URLs before display; currently nothing is stopping me from submitting <script>alert("foo");</script> as forge URL, and it gets interpreted when Datatable inserts it in the DOM. (or as a less malicious example, http://example.org/&=foo is displayed as http://example.org/&=foo

You can use a datatables render function to prevent XSS, see "Cross Site scripting" in this documentation.

cypress/support/index.js
64 ↗	(On Diff #26647)	that's a pain to type in the ui login form to connect though ¯\_(ツ)_/¯ ;)
swh/web/templates/add_forge_now/requests-moderation.html
17 ↗	(On Diff #26647)	Please, provide something actionable to fill in. Will that do? {"heading": "Add forge now requests moderation"}, @anlambert any issue with my proposal?
37 ↗	(On Diff #26647)	i did not get it, do you have something in mind to show in there? It's filled in when an issue occurs by the js.

In D7365#191782, @anlambert wrote:

And you need to sanitize forge URLs before display; currently nothing is stopping me from submitting <script>alert("foo");</script> as forge URL, and it gets interpreted when Datatable inserts it in the DOM. (or as a less malicious example, http://example.org/&=foo is displayed as http://example.org/&=foo

You can use a datatables render function to prevent XSS, see "Cross Site scripting" in this documentation.

Thanks, that's actionable.

anlambert added inline comments.Mar 17 2022, 3:52 PM

cypress/support/index.js
64 ↗	(On Diff #26647)	yes but you only need to do it once as you can save the credentials in your browser.
swh/web/templates/add_forge_now/requests-moderation.html
17 ↗	(On Diff #26647)	looks good to me

ardumont marked 4 inline comments as done.Mar 17 2022, 3:57 PM

ardumont added inline comments.

assets/src/bundles/add_forge/moderation-dashboard.js
20	true, i forgot ('cause i saw you mentioning it in the other diff) thanks.

vlorentz added inline comments.Mar 17 2022, 4:00 PM

swh/web/templates/add_forge_now/requests-moderation.html
37 ↗	(On Diff #26647)	"Could not load request list" or something It's filled in when an issue occurs by the js. Assuming the JS code is loaded, which is not guaranteed. See https://sentry.softwareheritage.org/organizations/swh/issues/?project=13&query=is%3Aunresolved+highlightjs&statsPeriod=14d for example, this shows how often the main JS code is successfully loaded but highlightjs isn't. I can guess the main JS code fails to load way more often than that, but we don't have the data to know that exactly.

In D7365#191788, @ardumont wrote:

In D7365#191782, @anlambert wrote:

And you need to sanitize forge URLs before display; currently nothing is stopping me from submitting <script>alert("foo");</script> as forge URL, and it gets interpreted when Datatable inserts it in the DOM. (or as a less malicious example, http://example.org/&=foo is displayed as http://example.org/&=foo

You can use a datatables render function to prevent XSS, see "Cross Site scripting" in this documentation.

Thanks, that's actionable.

By the way, we should probably punycode them as well, to avoid homoglyph attacks

Adapt according to review:

moderation view server: drop useless query since the datatable is providing the information
moderation view datatable: use a render text function to prevent xss attacks
moderation view: fill-in the heading variable
layout: keep the moderation entry menu short for consistency
rework commit message

ardumont retitled this revision from add-forge-now: Add moderation view to Add moderation view to list add-forge-now requests.Mar 17 2022, 4:04 PM

ardumont edited the summary of this revision. (Show Details)

ardumont added inline comments.Mar 17 2022, 4:07 PM

swh/web/templates/add_forge_now/requests-moderation.html
37 ↗	(On Diff #26647)	I've done nothing but aligned with a `<p>` like other views we have. So it's not really addressed but it's consistently unadressed. I guess we should open a task for that.

vlorentz added inline comments.Mar 17 2022, 4:08 PM

swh/web/templates/add_forge_now/requests-moderation.html
37 ↗	(On Diff #26647)	ok

vlorentz accepted this revision as: vlorentz.Mar 17 2022, 4:09 PM

ardumont added inline comments.Mar 17 2022, 4:10 PM

cypress/fixtures/add-forge-now-requests.json
11	@vlorentz by the way, i did amend that part as well ;)

vlorentz added inline comments.Mar 17 2022, 4:11 PM

cypress/fixtures/add-forge-now-requests.json
11	I still see email addresses with non-example domains though

anlambert accepted this revision.Mar 17 2022, 4:11 PM

This revision is now accepted and ready to land.Mar 17 2022, 4:11 PM

Build is green

Patch application report for D7365 (id=26648)

Could not rebase; Attempt merge onto 7bfbda85ff...

Merge made by the 'recursive' strategy.
 assets/src/bundles/add_forge/create-request.js     |  92 ++++
 assets/src/bundles/add_forge/index.js              |  11 +
 .../src/bundles/add_forge/moderation-dashboard.js  |  55 +++
 cypress/fixtures/add-forge-now-requests.json       |  79 ++++
 .../add-forge-now-request-create.spec.js           | 139 ++++++
 .../add-forge-now-requests-moderation.spec.js      | 122 ++++++
 cypress/integration/deposit-admin.spec.js          |  52 ++-
 cypress/support/index.js                           |   8 +
 swh/web/add_forge_now/__init__.py                  |   0
 swh/web/add_forge_now/apps.py                      |  10 +
 swh/web/add_forge_now/migrations/0001_initial.py   | 109 +++++
 swh/web/add_forge_now/migrations/__init__.py       |   0
 swh/web/add_forge_now/models.py                    |  99 +++++
 swh/web/add_forge_now/tests/test_migration.py      |  62 +++
 swh/web/add_forge_now/views.py                     | 107 +++++
 swh/web/admin/add_forge_now.py                     |  30 ++
 swh/web/admin/urls.py                              |   3 +-
 swh/web/api/urls.py                                |   1 +
 swh/web/api/views/add_forge_now.py                 | 355 +++++++++++++++
 swh/web/auth/utils.py                              |   1 +
 swh/web/common/utils.py                            |   6 +-
 swh/web/inbound_email/__init__.py                  |   0
 swh/web/inbound_email/apps.py                      |  11 +
 .../management/commands/process_inbound_email.py   |  73 ++++
 swh/web/inbound_email/signals.py                   |  36 ++
 swh/web/inbound_email/utils.py                     |  63 +++
 swh/web/settings/common.py                         |   2 +
 .../templates/add_forge_now/create-request.html    | 234 ++++++++++
 .../add_forge_now/requests-moderation.html         |  45 ++
 swh/web/templates/layout.html                      |  18 +-
 swh/web/tests/add_forge_now/test_models.py         |  26 ++
 swh/web/tests/add_forge_now/test_views.py          | 203 +++++++++
 swh/web/tests/api/views/test_add_forge_now.py      | 483 +++++++++++++++++++++
 swh/web/tests/conftest.py                          |  12 +-
 swh/web/tests/create_test_admin.py                 |  10 +-
 swh/web/tests/create_test_users.py                 |  23 +-
 .../tests/inbound_email/test_management_command.py | 161 +++++++
 swh/web/tests/inbound_email/test_utils.py          | 113 +++++
 swh/web/tests/utils.py                             |  14 +-
 swh/web/urls.py                                    |   1 +
 40 files changed, 2851 insertions(+), 18 deletions(-)
 create mode 100644 assets/src/bundles/add_forge/create-request.js
 create mode 100644 assets/src/bundles/add_forge/index.js
 create mode 100644 assets/src/bundles/add_forge/moderation-dashboard.js
 create mode 100644 cypress/fixtures/add-forge-now-requests.json
 create mode 100644 cypress/integration/add-forge-now-request-create.spec.js
 create mode 100644 cypress/integration/add-forge-now-requests-moderation.spec.js
 create mode 100644 swh/web/add_forge_now/__init__.py
 create mode 100644 swh/web/add_forge_now/apps.py
 create mode 100644 swh/web/add_forge_now/migrations/0001_initial.py
 create mode 100644 swh/web/add_forge_now/migrations/__init__.py
 create mode 100644 swh/web/add_forge_now/models.py
 create mode 100644 swh/web/add_forge_now/tests/test_migration.py
 create mode 100644 swh/web/add_forge_now/views.py
 create mode 100644 swh/web/admin/add_forge_now.py
 create mode 100644 swh/web/api/views/add_forge_now.py
 create mode 100644 swh/web/inbound_email/__init__.py
 create mode 100644 swh/web/inbound_email/apps.py
 create mode 100644 swh/web/inbound_email/management/commands/process_inbound_email.py
 create mode 100644 swh/web/inbound_email/signals.py
 create mode 100644 swh/web/inbound_email/utils.py
 create mode 100644 swh/web/templates/add_forge_now/create-request.html
 create mode 100644 swh/web/templates/add_forge_now/requests-moderation.html
 create mode 100644 swh/web/tests/add_forge_now/test_models.py
 create mode 100644 swh/web/tests/add_forge_now/test_views.py
 create mode 100644 swh/web/tests/api/views/test_add_forge_now.py
 create mode 100644 swh/web/tests/inbound_email/test_management_command.py
 create mode 100644 swh/web/tests/inbound_email/test_utils.py

Changes applied before test

commit 714010eca6cd77da80bc695c98ea1c50208663d3
Merge: 7bfbda85 7e61894e
Author: Jenkins user <jenkins@localhost>
Date:   Thu Mar 17 15:04:30 2022 +0000

    Merge branch 'diff-target' into HEAD

commit 7e61894e90438f53c625961350d92cd01dc62138
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Mar 17 10:14:51 2022 +0100

    Add moderation view to list add-forge-now requests
    
    In a future commit, it will be possible to browse from that view to the detailed request
    dashboard.
    
    This also reworks the left menu to allow access to staff or people with the add forge
    now moderator permission.
    
    Related to T3991

commit 4e23aa2eb8fca1f5c62a745f44f2f4b3b46003a9
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Mar 17 10:13:22 2022 +0100

    Fix access for people with permission access to deposit moderation
    
    This will allow easier adaptations when adding new views with different permissions.

commit 6c96f964adc7ffb7e9ea32de6d240a6a45bd8b97
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Thu Mar 17 11:33:57 2022 +0100

    Create "Add forge now" request
    
    Added a form to submit an "add forge now" request.
    Left menus changed to include "add forge now" page.
    Possbility to browse existing requests is added
    in a seperate tab in the same view.
    Cypress tests
    
    Related to T3998
    Related to T3989
    Related to T4012
    Related to T3988

commit 80e55b0a12eb27a5df72acf6fe07640db12eaf27
Merge: c24e6822 9f2e1cb9
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Wed Mar 16 16:21:35 2022 +0100

    Merge branch 'master' into sprint-add-forge-now

commit c24e6822a1569817b6d970fbc550cdf66f2188f0
Merge: ca39b06c 627ba79a
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Wed Mar 16 14:20:52 2022 +0100

    Merge branch 'master' into sprint-add-forge-now

commit ca39b06c46ba42f73f898377d3093299ea21a85a
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Wed Mar 9 16:37:54 2022 +0100

    inbound_email: add utilities to extract and filter email recipients

commit bf099ce9654d0b765f0820189857ab7c7beb6c6b
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Wed Mar 9 14:29:37 2022 +0100

    Bootstrap a common app for management of inbound emails

commit b6d540d2e5fafe04eb71d3b1d8f5c194dafcbb62
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Mar 15 15:06:10 2022 +0100

    add_forge_now: Move datatables listing in dedicated endpoint
    
    Merging datatables special processing in /api/1/add-forge/request/list
    endpoint implementation was quite disturbing.
    
    So better using a dedicated endpoint for that in a similar manner as
    with save code now requests listing.
    
    The endpoint to use with datatables is now the following one:
    /add-forge/request/list/datatables.
    
    Related to T3989
    Related to T3991

commit f6343395310bc226a50a283c88304006e8ece337
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Mon Mar 14 15:35:17 2022 +0100

    api/add_forge_now: Allow to use requests list endpoint with datatables
    
    datatables is the javascript library we use on the frontend side to
    display interactive tables.
    
    As we use server-side processing, table data must be provided in a
    paginated way by an HTTP endpoint.
    
    Response format expected by datatables is different from the one
    returned by the Web API endpoint listing add-forge requests.
    
    So adapt the response format of that endpoint when we know the
    input request has been sent by datatables.
    
    Related to T3989
    Related to T3991

commit 26748e56ecce5877fce3a215eba06a19ebb8342f
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Thu Mar 10 14:09:36 2022 +0100

    api: Add endpoint to get details about an add-forge request
    
    Related to T4030

commit 294a95c711bc40622c6d718afa2535f377b90dfc
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Mar 9 16:30:06 2022 +0100

    api: Add endpoint to list add-forge requests
    
    Related to T4027

commit 1bf17d6a75fd7265b1da9c9e87220efa62869c93
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Mar 9 16:28:03 2022 +0100

    api: Add endpoint to update an add-forge request
    
    Related to T4026

commit 130e9faa9bcab18bc3c76dc898546edb89f3f9b8
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Mar 8 15:23:50 2022 +0100

    api: Add endpoint to create an add-forge request
    
    Related to T3990

commit 03101208803501e5178f35d18d171e740ee4ca76
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Mar 8 11:34:44 2022 +0100

    add_forge_now: Bootstrap app and model

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1488/ for more details.

Harbormaster completed remote builds in B27531: Diff 26648.Mar 17 2022, 4:17 PM

ardumont added inline comments.Mar 17 2022, 4:37 PM

cypress/fixtures/add-forge-now-requests.json
11	true. I only changed the swh-web.org ones, it did not occur to me to do more ¯\_(ツ)_/¯. In general, do you want all those falsy emails we got to be `@example.org`? I'm more inclined to name them according to the test but i'm fine either way. (To be clear though, i won't change this more though, i don't want to wait again for builds to finish).

Closed by commit rDWAPPS7e61894e9043: Add moderation view to list add-forge-now requests (authored by ardumont). · Explain WhyMar 17 2022, 4:39 PM

This revision was automatically updated to reflect the committed changes.

ardumont added a commit: rDWAPPS7e61894e9043: Add moderation view to list add-forge-now requests.

vlorentz mentioned this in D7454: Save Forge Now: Fix XSS in request list.Mar 29 2022, 2:00 PM

Add moderation view to list add-forge-now requests
ClosedPublic
Actions

Details

Diff Detail

Unit TestsFailed
View All

Event Timeline

Patch application report for D7365 (id=26627)

Changes applied before test

Patch application report for D7365 (id=26628)

Changes applied before test

Patch application report for D7365 (id=26631)

Changes applied before test

Patch application report for D7365 (id=26647)

Changes applied before test

Patch application report for D7365 (id=26648)

Changes applied before test

Revision Contents
Changeset List

Diff 26628

assets/src/bundles/add_forge/index.js

assets/src/bundles/add_forge/moderation-dashboard.js

cypress/fixtures/add-forge-now-requests.json

cypress/integration/add-forge-now-moderation.spec.js

swh/web/add_forge_now/views.py

swh/web/common/utils.py

swh/web/templates/add_forge_now/moderation.html

swh/web/templates/layout.html

swh/web/tests/create_test_users.py

Add moderation view to list add-forge-now requestsClosedPublicActions

Details

Diff Detail

Unit TestsFailedView All

Event Timeline

Patch application report for D7365 (id=26627)

Changes applied before test

Patch application report for D7365 (id=26628)

Changes applied before test

Patch application report for D7365 (id=26631)

Changes applied before test

Patch application report for D7365 (id=26647)

Changes applied before test

Patch application report for D7365 (id=26648)

Changes applied before test

Revision ContentsChangeset List

Diff 26628

assets/src/bundles/add_forge/index.js

assets/src/bundles/add_forge/moderation-dashboard.js

cypress/fixtures/add-forge-now-requests.json

cypress/integration/add-forge-now-moderation.spec.js

swh/web/add_forge_now/views.py

swh/web/common/utils.py

swh/web/templates/add_forge_now/moderation.html

swh/web/templates/layout.html

swh/web/tests/create_test_users.py

Add moderation view to list add-forge-now requests
ClosedPublic
Actions

Unit TestsFailed
View All

Revision Contents
Changeset List