Page MenuHomeSoftware Heritage

Search inside the origin releases with name
ClosedPublic

Authored by jayeshv on Jun 29 2021, 10:42 AM.

Details

Summary

This change enables the possibility to search with release names inside
the browse origin releases page.A search textbox is added, which is
connected to a JS function and that reloads the page with the
release_name_include query param.
Filtering with the name is happening at the storage level.

Related to T3404

Test Plan

Go to an archived origin and to its Releases tab. eg: https://archive.softwareheritage.org/browse/origin/releases/?origin_url=https://github.com/SoftwareHeritage/swh-model

  • search for a releases, eg: v2
    • page will reload with only matching releases
    • url will have search string as query param
    • the search text will be pre-populated with the search term
  • Search for a non existing release name
    • "No release names containing 'search term' have been found!" message will appear
  • Search with empty release name (or delete the text in search box and enter)
    • Page with all the release will appear

Diff Detail

Repository
rDWAPPS Web applications
Branch
T3404
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 22307
Build 34720: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 34719: arc lint + arc unit

Event Timeline

jayeshv added a reviewer: anlambert.

Build is green

Patch application report for D5939 (id=21325)

Rebasing onto e18341dd82...

Current branch diff-target is up to date.
Changes applied before test
commit 3e27f71502764739f93895863f0a87a53edb7a13
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Jun 29 10:35:55 2021 +0200

    Search inside the origin releases with name
    
    This change enables the possibility to search with release names inside
    the browse origin releases page.A search textbox is added, which is
    connected to a JS function and that reloads the page with the
    release_name_include query param.
    Filtering with the name is happening at the storage level.
    
    Related to T3404

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

Looks good but some code could be deduplicated before landing this. I also forgot to tell you during last review that we must update copyright headers years when modifying source files (see inline comments).

assets/src/bundles/browse/browse-utils.js
2

2018-2021

cypress/integration/origin-browse.spec.js
2

2020-2021

swh/web/browse/snapshot_context.py
1

2018-2021

swh/web/templates/browse/releases.html
4

2017-2021

13–23

In order to avoid duplicating that HTML code, you should put it in a dedicated HTML file in the templates/includes folder and include it in branches and releases templates with a {% include ... %} directive.

You can make the code generic the following way:

<div class="form-group row float-right">
  <form class="form-horizontal d-none d-md-flex input-group" id="swh-snapshot-search-form">
    <input class="form-control" placeholder="{{ search_placeholder }}"
      id="swh-snapshot-search-string" value="{{search_string}}" type="text" />
    <div class="input-group-append">
      <button class="btn btn-primary" type="submit" id="swh-snapshot-serach-button">
        <i class="swh-search-icon mdi mdi-24px mdi-magnify" aria-hidden="true"></i>
      </button>
    </div>
  </form>
</div>

This will also enable to not duplicate the JavaScript submit handler but you need to rename the query parameter to name_include to work with both views.

34–69

Lot of indentation issues here. We should try to find a HTML formatter that works with django template.

This revision now requires changes to proceed.Jun 29 2021, 11:09 AM

Build is green

Patch application report for D5939 (id=21326)

Rebasing onto e18341dd82...

Current branch diff-target is up to date.
Changes applied before test
commit 5f4e761d150e6cefa0fd1158d68c35e5857dba0c
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Jun 29 11:01:45 2021 +0200

    Search inside the origin releases with name
    
    This change enables the possibility to search with release names inside
    the browse origin releases page.A search textbox is added, which is
    connected to a JS function and that reloads the page with the
    release_name_include query param.
    Filtering with the name is happening at the storage level.
    
    Related to T3404

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

template changes, removed tabs

swh/web/templates/browse/releases.html
34–69

The problem was jija2-mode was adding some tab chars. I fixed it now

swh/web/templates/browse/releases.html
34–69

I am trying to find a good jinja2 formatter that we could use with pre-commit like the black tool for Python code, did not find a good candidate so far.

swh/web/templates/browse/releases.html
34–69

jinja-2 mode in emacs works somewhat well. I am adding some conf to avoid TAB chars.
But this mode has issues with self closing tags.

Build is green

Patch application report for D5939 (id=21328)

Rebasing onto e18341dd82...

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

    Search inside the origin releases with name
    
    This change enables the possibility to search with release names inside
    the browse origin releases page.A search textbox is added, which is
    connected to a JS function and that reloads the page with the
    release_name_include query param.
    Filtering with the name is happening at the storage level.
    
    Related to T3404

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

swh/web/templates/browse/releases.html
34–69

js-beautify seems a good candidate as it supports common template languages and can be finely tuned.

Build is green

Patch application report for D5939 (id=21332)

Rebasing onto e18341dd82...

Current branch diff-target is up to date.
Changes applied before test
commit 8024e89d4445082b0ff63d76ead1c76faad63e28
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Jun 29 11:58:25 2021 +0200

    Search inside the origin releases with name
    
    This change enables the possibility to search with release names inside
    the browse origin releases page.A search textbox is added, which is
    connected to a JS function and that reloads the page with the
    release_name_include query param.
    Filtering with the name is happening at the storage level.
    
    Related to T3404

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

Looks good to me !

cypress/integration/origin-browse.spec.js
110

should show all branches for empty search

150

should show all the releases for empty search

swh/web/templates/browse/releases.html
34–69

js-beautify could also be used to format js code in a consistent way but we need to adapt eslint rules for the both tool to play well, need to do more experiments on the subject.

This revision is now accepted and ready to land.Jun 29 2021, 2:08 PM
This revision was landed with ongoing or failed builds.Jun 29 2021, 2:28 PM
This revision was automatically updated to reflect the committed changes.

Build is green

Patch application report for D5939 (id=21347)

Rebasing onto e18341dd82...

Current branch diff-target is up to date.
Changes applied before test
commit e26ceb221a88da81358110f4e1eb824de2a33021
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Jun 29 14:23:19 2021 +0200

    Search inside the origin releases with name
    
    This change enables the possibility to search with release names inside
    the browse origin releases page.A search textbox is added, which is
    connected to a JS function and that reloads the page with the
    release_name_include query param.
    Filtering with the name is happening at the storage level.
    
    Related to T3404

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