Page MenuHomeSoftware Heritage

Redirect to the browse origin page when the user searchs for an exact, already archived URL
ClosedPublic

Authored by jayeshv on Jun 8 2021, 7:14 PM.

Details

Summary

When the user searches with a valid, archived URL, it makes more sense to redirect directly to the browse page rather than showing search results.
Two generic functions added are 'isValidURL' and 'isArchivedOrigin'.
First one returns a boolean whereas the latter returns a promise.

Related to T3354

Diff Detail

Repository
rDWAPPS Web applications
Branch
T3354
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 21983
Build 34196: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 34195: arc lint + arc unit

Event Timeline

Build has FAILED

Patch application report for D5828 (id=20849)

Rebasing onto 89c0111746...

First, rewinding head to replay your work on top of it...
Applying: Added utility function to identify a valid url in the search input
Applying: Added function to check whether the given URL is alrady archived
Applying: Added a fuction 'isArchivedOrigin', that returns a promise.
Applying: Moved the isArchivedOrigin function to utils/functions.js
Applying: Setting the window.location to the searched origin
Changes applied before test
commit a21eb740e903a276493b881ab77c2522bb738d07
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Jun 8 18:18:27 2021 +0200

    Setting the window.location to the searched origin

commit ec91da970cf6c08909ecd8fa77884bdc55226d2e
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Jun 8 17:48:56 2021 +0200

    Moved the isArchivedOrigin function to utils/functions.js

commit 7b3df48d14b0ad97801701b8892094ed699c50b0
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Jun 8 14:42:46 2021 +0200

    Added a fuction 'isArchivedOrigin', that returns a promise.

commit 77df9f5cf70680b2b18c82c45f6a6efca2ee90bb
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Mon Jun 7 18:36:26 2021 +0200

    Added function to check whether the given URL is alrady archived

commit 7dee8278146167723a58ab74d884771ebefc6913
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Mon Jun 7 17:52:35 2021 +0200

    Added utility function to identify a valid url in the search input

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

Harbormaster returned this revision to the author for changes because remote builds failed.Jun 8 2021, 7:30 PM
Harbormaster failed remote builds in B21811: Diff 20849!

Build has FAILED

Patch application report for D5828 (id=20849)

Rebasing onto da39599f34...

First, rewinding head to replay your work on top of it...
Applying: Added utility function to identify a valid url in the search input
Applying: Added function to check whether the given URL is alrady archived
Applying: Added a fuction 'isArchivedOrigin', that returns a promise.
Applying: Moved the isArchivedOrigin function to utils/functions.js
Applying: Setting the window.location to the searched origin
Changes applied before test
commit 03ed654798ac5521ee726d7c09d5e681c0f32a33
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Jun 8 18:18:27 2021 +0200

    Setting the window.location to the searched origin

commit cb1d4a1f51b7afa0e273d92fb303e2ef35fe91d5
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Jun 8 17:48:56 2021 +0200

    Moved the isArchivedOrigin function to utils/functions.js

commit bf4a6340c558cf6faf24a73e067964c4194cd5cd
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Jun 8 14:42:46 2021 +0200

    Added a fuction 'isArchivedOrigin', that returns a promise.

commit 3a4e68dbfd5eddd33c551c782c6f68a5f7a499c0
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Mon Jun 7 18:36:26 2021 +0200

    Added function to check whether the given URL is alrady archived

commit eb27f1475639f95f022235069cbf68411e8d389c
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Mon Jun 7 17:52:35 2021 +0200

    Added utility function to identify a valid url in the search input

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

Build is green

Patch application report for D5828 (id=20992)

Could not rebase; Attempt merge onto a0db251b32...

Updating a0db251b..a2b84369
Fast-forward
 assets/src/bundles/browse/origin-search.js | 19 ++++++++----
 assets/src/utils/functions.js              | 32 ++++++++++++++++++++
 cypress/integration/origin-search.spec.js  | 47 +++++++++++++++++++-----------
 3 files changed, 75 insertions(+), 23 deletions(-)
Changes applied before test
commit a2b84369d7cf64a9d799a90ffd9d8ef3bdd68cd4
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Mon Jun 14 15:26:03 2021 +0200

    Added tests for non valid URL search and valid non archived URL search

commit c2c1979d7cc0473454fc92bdad783ed3ae7f308d
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Mon Jun 14 15:15:44 2021 +0200

    Added tests for redirect while searching with archived URL

commit 06cf25ec6680a639c5b1918ee66f75aac6f11bba
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Mon Jun 14 12:25:10 2021 +0200

    Removed failing test cases

commit b03af8c8434e25b2bd7cdb6ca46a788a89ac3fba
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Jun 8 18:18:27 2021 +0200

    Setting the window.location to the searched origin

commit 7175382e84998bc2ccee36148f0549656e19a0e5
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Jun 8 17:48:56 2021 +0200

    Moved the isArchivedOrigin function to utils/functions.js

commit 7cc2b00d61343b7b0d607a34ac89282dfda7d16f
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Jun 8 14:42:46 2021 +0200

    Added a fuction 'isArchivedOrigin', that returns a promise.

commit b1593d60de46b7e5729549a5fa6ce92952f59d28
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Mon Jun 7 18:36:26 2021 +0200

    Added function to check whether the given URL is alrady archived

commit 7808951a31e959826e6bfabffc0f81999b396c59
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Mon Jun 7 17:52:35 2021 +0200

    Added utility function to identify a valid url in the search input

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

Updating to changes from origin/master

Build is green

Patch application report for D5828 (id=20993)

Rebasing onto a0db251b32...

Current branch diff-target is up to date.
Changes applied before test
commit a2b84369d7cf64a9d799a90ffd9d8ef3bdd68cd4
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Mon Jun 14 15:26:03 2021 +0200

    Added tests for non valid URL search and valid non archived URL search

commit c2c1979d7cc0473454fc92bdad783ed3ae7f308d
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Mon Jun 14 15:15:44 2021 +0200

    Added tests for redirect while searching with archived URL

commit 06cf25ec6680a639c5b1918ee66f75aac6f11bba
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Mon Jun 14 12:25:10 2021 +0200

    Removed failing test cases

commit b03af8c8434e25b2bd7cdb6ca46a788a89ac3fba
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Jun 8 18:18:27 2021 +0200

    Setting the window.location to the searched origin

commit 7175382e84998bc2ccee36148f0549656e19a0e5
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Jun 8 17:48:56 2021 +0200

    Moved the isArchivedOrigin function to utils/functions.js

commit 7cc2b00d61343b7b0d607a34ac89282dfda7d16f
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Jun 8 14:42:46 2021 +0200

    Added a fuction 'isArchivedOrigin', that returns a promise.

commit b1593d60de46b7e5729549a5fa6ce92952f59d28
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Mon Jun 7 18:36:26 2021 +0200

    Added function to check whether the given URL is alrady archived

commit 7808951a31e959826e6bfabffc0f81999b396c59
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Mon Jun 7 17:52:35 2021 +0200

    Added utility function to identify a valid url in the search input

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

Looks good but can be improved a little.

Also please squash all commits into a single one prior landing this.

assets/src/utils/functions.js
90–111

Could you use the async/await syntax of modern javascript in that function instead ?

This way you could return a boolean instead to simplify newly added code in doSearch function.

For the record, I am currently in the process of migrating all code using promises to async/await as it improves readability.

This revision now requires changes to proceed.Jun 14 2021, 5:18 PM
assets/src/utils/functions.js
90–111

For the record, I am currently in the process of migrating all code using promises to async/await as it improves readability.

D5866

assets/src/utils/functions.js
90–111

Thanks @anlambert . I will be changing this. It is much more readable with await

Changed Promise to use async/await.

Build is green

Patch application report for D5828 (id=21004)

Rebasing onto a0db251b32...

Current branch diff-target is up to date.
Changes applied before test
commit 17a5710fae456ab5fc8c018d6167c87a0162a90d
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Mon Jun 14 20:30:37 2021 +0200

    T3354: When an archived URL is searched, user is redirected to the origin browse page directly

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

jayeshv marked an inline comment as not done.

Synatax corrections

Build is green

Patch application report for D5828 (id=21005)

Rebasing onto a0db251b32...

Current branch diff-target is up to date.
Changes applied before test
commit 6b1a48488d097d49f8b8337d7dcb87a49542ec31
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Mon Jun 14 22:17:09 2021 +0200

    T3354: When an archived URL is searched, user is redirected to the origin browse page directly

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

Almost there, more code can be simplified and commit message should be improved.

At SWH, we tend to adhere to this commit message guidelines
(personally I used at most 72 characters for commit title and try to wrap body to 80 characters).
I suggest you to reuse the diff summary as commit message body.
Also you should reference the task that led to that commit at the end of the commit message body
using the following sentence: Related to T3354, phabricator will then automatically link task and
commit (more info in documentation).

assets/src/utils/functions.js
91–111

You should also simplify the code of that function with async/await, something like:

export async function isArchivedOrigin(originPath) {
  if (!isValidURL(originPath)) {
    // Not a valid URL, return immediately
    return false;
  } else {
    const response = await fetch(Urls.api_1_origin(originPath));
    return response.ok && response.status === 200;
  }
}
This revision now requires changes to proceed.Jun 15 2021, 11:10 AM

Incorporating suggested changes and improved commit message

Build is green

Patch application report for D5828 (id=21018)

Rebasing onto a0db251b32...

Current branch diff-target is up to date.
Changes applied before test
commit 740ad9ba51c73533ee009c305e120e373cd984be
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Jun 15 11:41:37 2021 +0200

    Redirect any archived URL search to the browse origin page
    
    When the user searches with a valid, archived URL, it makes more sense
    to redirect directly to the browse page rather than showing search
    results.Two generic functions added are 'isValidURL' and
    'isArchivedOrigin'. Both returns boolean.

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

Looks good to me ! You can now merge your feature branch in the master one and push it to remote, phabricator will automatically close the diff afterwards.

This revision is now accepted and ready to land.Jun 15 2021, 12:24 PM

Looks good to me ! You can now merge your feature branch in the master one and push it to remote, phabricator will automatically close the diff afterwards.

Forgot to add the related ticket name. I hope I can close manually.