⚙ D7489 added hyperlinks to URLs in Browse Requests tab

anirudhlakhotia created this revision.Apr 3 2022, 1:13 PM

Herald added a required legal document: L3 Software Heritage Contributor License Agreement, version 1.0. · View Herald TranscriptApr 3 2022, 1:13 PM

Build has FAILED

Patch application report for D7489 (id=27162)

Rebasing onto ad5add7d36...

Current branch diff-target is up to date.

Changes applied before test

commit d505a441e26070e637a28295ed303f3fbb6646ff
Author: anirudhlakhotia <sanjeev196945@gmail.com>
Date:   Sun Apr 3 14:26:46 2022 +0530

    added hyperlinks to URLs in Browse Requests tab

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

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 3 2022, 1:15 PM

Harbormaster failed remote builds in B28077: Diff 27162!

Great, thanks for the diff.

Can you please:

add the Related to T4078 at the end of the description (the bottom line)? That will help link diff to task.

read and eventually accept the L3 document? Otherwise, we won't be able to review the content of your diff.

finally, make sure the build stays green? [1]

[1] For that bit, either look at the comment D7489#195670 (link console output to check the logs) or even faster make the tests run locally to check what's wrong locally.

Cheers,

anirudhlakhotia edited the summary of this revision. (Show Details)Apr 3 2022, 5:05 PM

anirudhlakhotia added a task: T4078: add forge now - add a link on forge url.

Minor changes to the diff

Build is green

Patch application report for D7489 (id=27166)

Could not rebase; Attempt merge onto ad5add7d36...

Updating ad5add7d..06669f58
Fast-forward
 assets/src/bundles/add_forge/create-request.js | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Changes applied before test

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/1654/ for more details.

Harbormaster completed remote builds in B28081: Diff 27166.Apr 3 2022, 5:50 PM

anirudhlakhotia requested review of this revision.Apr 3 2022, 5:50 PM

Thanks for the re-work.

It's missing your initial content of the diff though.

I believe your arc diff update was wrong. I only see the last commit you did about your minor change.

It should have been something like (with 2 commits):

arc diff HEAD~2 --update D7489

But here, the 2nd commit is (drop spurious empty line) is not needed as a commit.
Just squash the initial commit and this one into the original commit and the update the diff.
And then the command to update it just becomes:

arc diff HEAD~ --update D7489

Cheers,

Added hyperlinks to the URLs in Browse Requests tab

Added Hyperlinks to URLs in the Browse Requests Tab

Build has FAILED

Patch application report for D7489 (id=27173)

Rebasing onto 8ffd81760a...

First, rewinding head to replay your work on top of it...
Applying: added hyperlinks to URLs in Browse Requests tab

Changes applied before test

commit 51cbac305b6bf9dcbbc2c19c2ef18e134af58c07
Author: anirudhlakhotia <sanjeev196945@gmail.com>
Date:   Sun Apr 3 14:26:46 2022 +0530

    added hyperlinks to URLs in Browse Requests tab

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

Harbormaster failed remote builds in B28085: Diff 27173!Apr 4 2022, 1:40 PM

Build is green

Patch application report for D7489 (id=27172)

Rebasing onto 8ffd81760a...

First, rewinding head to replay your work on top of it...
Applying: Ensure that tests run with the C.UTF-8 locale
Using index info to reconstruct a base tree...
M	swh/web/tests/conftest.py
Falling back to patching base and 3-way merge...
No changes -- Patch already applied.
Applying: added hyperlinks to URLs in Browse Requests tab

Changes applied before test

commit 4f9304df333c1df63f439fdb008cef97027ee1c6
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/1657/ for more details.

Harbormaster completed remote builds in B28084: Diff 27172.Apr 4 2022, 1:43 PM

Thanks for your contribution. I have added some inline comments to handle before we can land this.

assets/src/bundles/add_forge/create-request.js
122–133	You need to sanitize the URL to prevent XSS attacks, see example here. Also, when linking to an external site in a table column displaying an URL, we prefer to add a clickable "open in new" icon that will open the URL in a new browser tab (see example here).
swh/web/tests/conftest.py
59 ↗	(On Diff #27172)	Why this change ?

This revision now requires changes to proceed.Apr 4 2022, 1:43 PM

ardumont added inline comments.Apr 4 2022, 1:47 PM

swh/web/tests/conftest.py
59 ↗	(On Diff #27172)	see #swh-devel, it's the wrong range commit. It's no longer there now if you reload the page.

ardumont added inline comments.Apr 4 2022, 1:48 PM

swh/web/tests/conftest.py
59 ↗	(On Diff #27172)	As for the reason, see the original diff [1] [1] D7483

anirudhlakhotia added inline comments.Apr 4 2022, 1:54 PM

swh/web/tests/conftest.py
59 ↗	(On Diff #27172)	I'm not really sure how that changed, I thought it was a change I had pulled before updating the diff, I checked it now, and it isn't a necessary change. Sorry about that, the change can be discarded.

Build is green

Patch application report for D7489 (id=27173)

Rebasing onto 8ffd81760a...

First, rewinding head to replay your work on top of it...
Applying: added hyperlinks to URLs in Browse Requests tab

Changes applied before test

commit 335e3db9b86c7e8f6a117cfb51a9c807c2401e52
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/1659/ for more details.

Harbormaster completed remote builds in B28085: Diff 27173.Apr 4 2022, 2:00 PM

Sanitized URLs, and switched to a clickable to open the link in a new tab

Looks good to me. Please add your name in the CONTRIBUTORS file at the root of the repository and I will accept the diff.

This revision now requires changes to proceed.Apr 4 2022, 2:38 PM

Build is green

Patch application report for D7489 (id=27177)

Could not rebase; Attempt merge onto 8ffd81760a...

Merge made by the 'recursive' strategy.
 assets/src/bundles/add_forge/create-request.js | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Changes applied before test

commit 39121735cb5b59d256ca51a58ab851a92caf9e32
Merge: 8ffd8176 05534635
Author: Jenkins user <jenkins@localhost>
Date:   Mon Apr 4 12:35:47 2022 +0000

    Merge branch 'diff-target' into HEAD

commit 05534635941460e50acc8caace7d51ebd7b31d29
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/1660/ for more details.

Harbormaster completed remote builds in B28089: Diff 27177.Apr 4 2022, 2:54 PM

Added Hyperlinks to the URLs in the Browse Requests tab

anirudhlakhotia marked an inline comment as done.Apr 4 2022, 2:59 PM

Still one change required before I can accept the diff (see inline comment).

CONTRIBUTORS
10 ↗	(On Diff #27179)	Contributors are sorted in lexicographical order so you must add your name on top of that list.

This revision now requires changes to proceed.Apr 4 2022, 3:11 PM

Build is green

Patch application report for D7489 (id=27179)

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 13800c9c28a7d97172b892c825244414bd123456
Merge: 8ffd8176 7651ce4e
Author: Jenkins user <jenkins@localhost>
Date:   Mon Apr 4 12:58:09 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/1661/ for more details.

Harbormaster completed remote builds in B28090: Diff 27179.Apr 4 2022, 3:18 PM

Added Hyperlinks to URLs in the browse requests tab

anirudhlakhotia marked an inline comment as done.Apr 4 2022, 3:25 PM

Build is green

Patch application report for D7489 (id=27186)

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 885253ff352ab03c23692d1469e09eda7437db20
Merge: 8ffd8176 a6f6906e
Author: Jenkins user <jenkins@localhost>
Date:   Mon Apr 4 13:25:23 2022 +0000

    Merge branch 'diff-target' into HEAD

commit a6f6906eb8ceaf88172fb475be909b7074dbd734
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/1664/ for more details.

Harbormaster completed remote builds in B28093: Diff 27186.Apr 4 2022, 3:44 PM

One last thing before we can land this:

please squash commits into a single one if its is not already done and use the following message:

add_forge_now: Add hyperlinks to forge URLs in Browse Requests tab

Related to T4078

rebase the diff on top of the master branch (tip commit is currently 8ffd81760aa971773b693a07934d717dfaf91b22) and update it

This revision now requires changes to proceed.Apr 4 2022, 3:57 PM

anirudhlakhotia edited the summary of this revision. (Show Details)Apr 4 2022, 4:33 PM

add_forge_now: Add hyperlinks to forge URLs in Browse Requests tab

Granted for landing, thanks !

This revision is now accepted and ready to land.Apr 4 2022, 5:00 PM

This revision was landed with ongoing or failed builds.Apr 4 2022, 5:02 PM

Closed by commit rDWAPPS902039b683f3: add_forge_now: Add hyperlinks to forge URLs in Browse Requests tab (authored by anirudhlakhotia). · Explain Why

This revision was automatically updated to reflect the committed changes.

anirudhlakhotia added a commit: rDWAPPS902039b683f3: add_forge_now: Add hyperlinks to forge URLs in Browse Requests tab.

Build is green

Patch application report for D7489 (id=27199)

Rebasing onto 8ffd81760a...

Current branch diff-target is up to date.

Changes applied before test

commit ed01faa9de9a506922c5053d68964ce18f06b2a9
Author: anirudhlakhotia <sanjeev196945@gmail.com>
Date:   Sun Apr 3 14:26:46 2022 +0530

    add_forge_now: Add hyperlinks to forge URLs in Browse Requests tab
    
    Related to T4078

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

Harbormaster completed remote builds in B28102: Diff 27199.Apr 4 2022, 5:24 PM

anirudhlakhotia mentioned this in T4246: URLs on moderation dashboard aren't sanitized.May 14 2022, 4:37 PM

added hyperlinks to URLs in Browse Requests tab
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Patch application report for D7489 (id=27162)

Changes applied before test

Patch application report for D7489 (id=27166)

Changes applied before test

Patch application report for D7489 (id=27173)

Changes applied before test

Patch application report for D7489 (id=27172)

Changes applied before test

Patch application report for D7489 (id=27173)

Changes applied before test

Patch application report for D7489 (id=27177)

Changes applied before test

Patch application report for D7489 (id=27179)

Changes applied before test

Patch application report for D7489 (id=27186)

Changes applied before test

Patch application report for D7489 (id=27199)

Changes applied before test

Revision Contents
Changeset List

Diff 27177

assets/src/bundles/add_forge/create-request.js

added hyperlinks to URLs in Browse Requests tabClosedPublicActions

Details

Diff Detail

Event Timeline

Patch application report for D7489 (id=27162)

Changes applied before test

Patch application report for D7489 (id=27166)

Changes applied before test

Patch application report for D7489 (id=27173)

Changes applied before test

Patch application report for D7489 (id=27172)

Changes applied before test

Patch application report for D7489 (id=27173)

Changes applied before test

Patch application report for D7489 (id=27177)

Changes applied before test

Patch application report for D7489 (id=27179)

Changes applied before test

Patch application report for D7489 (id=27186)

Changes applied before test

Patch application report for D7489 (id=27199)

Changes applied before test

Revision ContentsChangeset List

Diff 27177

assets/src/bundles/add_forge/create-request.js

added hyperlinks to URLs in Browse Requests tab
ClosedPublic
Actions

Revision Contents
Changeset List