Page MenuHomeSoftware Heritage

added hyperlinks to URLs in Browse Requests tab
ClosedPublic

Authored by anirudhlakhotia on Apr 3 2022, 1:13 PM.

Diff Detail

Repository
rDWAPPS Web applications
Branch
request-link
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 28089
Build 43992: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 43991: arc lint + arc unit

Event Timeline

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,

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.

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

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.

anlambert added a subscriber: anlambert.

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
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.

swh/web/tests/conftest.py
59 ↗(On Diff #27172)

As for the reason, see the original diff [1]

[1] D7483

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.

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.

Added Hyperlinks to the URLs in the Browse Requests tab

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.

  • Added Hyperlinks to URLs in the browse requests tab

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.

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
  • 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
This revision was automatically updated to reflect the committed changes.

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.