Page MenuHomeSoftware Heritage
Differential D5828

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

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

Details

Reviewers
anlambert
Group Reviewers
Reviewers
Maniphest Tasks
T3354: Search UI: redirect to browse page if input matches an archived origin URL
Commits
rDWAPPS740ad9ba51c7: Redirect any archived URL search to the browse origin page
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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jayeshv created this revision.Jun 8 2021, 7:14 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptJun 8 2021, 7:14 PM
swh-public-ci added a comment.Jun 8 2021, 7:30 PM

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!
jayeshv added a reviewer: anlambert.Jun 10 2021, 8:28 AM
swh-public-ci added a comment.Jun 10 2021, 8:47 AM

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

jayeshv updated this revision to Diff 20992.Jun 14 2021, 3:31 PM

Added frontend tests

swh-public-ci added a comment.Jun 14 2021, 3:40 PM

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.

Harbormaster completed remote builds in B21971: Diff 20992.Jun 14 2021, 3:40 PM
jayeshv requested review of this revision.Jun 14 2021, 3:40 PM
jayeshv updated this revision to Diff 20993.Jun 14 2021, 3:43 PM

Updating to changes from origin/master

swh-public-ci added a comment.Jun 14 2021, 3:51 PM

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.

Harbormaster completed remote builds in B21972: Diff 20993.Jun 14 2021, 3:51 PM
anlambert requested changes to this revision.Jun 14 2021, 5:18 PM

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
anlambert added inline comments.Jun 14 2021, 6:03 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

jayeshv added inline comments.Jun 14 2021, 8:16 PM
assets/src/utils/functions.js
90–111

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

jayeshv updated this revision to Diff 21004.Jun 14 2021, 8:31 PM

Changed Promise to use async/await.

jayeshv marked 2 inline comments as done.Jun 14 2021, 8:33 PM
swh-public-ci added a comment.Jun 14 2021, 8:41 PM

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.

Harbormaster completed remote builds in B21982: Diff 21004.Jun 14 2021, 8:41 PM
jayeshv updated this revision to Diff 21005.Jun 14 2021, 10:18 PM
jayeshv marked an inline comment as not done.

Synatax corrections

swh-public-ci added a comment.Jun 14 2021, 10:28 PM

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.

Harbormaster completed remote builds in B21983: Diff 21005.Jun 14 2021, 10:28 PM
anlambert edited the summary of this revision. (Show Details)Jun 15 2021, 11:06 AM
anlambert requested changes to this revision.Jun 15 2021, 11:10 AM

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
90–110

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
jayeshv updated this revision to Diff 21018.Jun 15 2021, 11:45 AM

Incorporating suggested changes and improved commit message

jayeshv marked an inline comment as done.Jun 15 2021, 11:47 AM
swh-public-ci added a comment.Jun 15 2021, 11:54 AM

Build was aborted

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

Harbormaster failed remote builds in B21996: Diff 21018!Jun 15 2021, 11:54 AM
swh-public-ci added a comment.Jun 15 2021, 12:15 PM

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.

Harbormaster completed remote builds in B21996: Diff 21018.Jun 15 2021, 12:15 PM
anlambert accepted this revision.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.

This revision is now accepted and ready to land.Jun 15 2021, 12:24 PM
jayeshv added a comment.Jun 15 2021, 12:41 PM
In D5828#149939, @anlambert wrote:

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.

Closed by commit rDWAPPS740ad9ba51c7: Redirect any archived URL search to the browse origin page (authored by jayeshv). · Explain WhyJun 15 2021, 12:45 PM
This revision was automatically updated to reflect the committed changes.
jayeshv added a commit: rDWAPPS740ad9ba51c7: Redirect any archived URL search to the browse origin page.

Revision Contents
Changeset List

PathSize
assets/
src/
bundles/
browse/
11 lines
utils/
19 lines
cypress/
integration/
47 lines

Diff 21026

View Options

assets/src/bundles/browse/origin-search.js

Loading...
View Options

assets/src/utils/functions.js

Loading...
View Options

cypress/integration/origin-search.spec.js

Loading...
About · Developer information · Legal