-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74165/#review224791
-----------------------------------------------------------




dashboardv2/public/js/collection/VRelationshipSearchList.js
Lines 20 (patched)
<https://reviews.apache.org/r/74165/#comment313585>

    remove Globals, not used in this file.



dashboardv2/public/js/collection/VRelationshipSearchList.js
Lines 24 (patched)
<https://reviews.apache.org/r/74165/#comment313586>

    remove GLobals



dashboardv2/public/js/collection/VRelationshipSearchResultList.js
Lines 20 (patched)
<https://reviews.apache.org/r/74165/#comment313587>

    remove Globals, not used in this file



dashboardv2/public/js/collection/VRelationshipSearchResultList.js
Lines 24 (patched)
<https://reviews.apache.org/r/74165/#comment313588>

    remove GLobals



dashboardv2/public/js/collection/VRelationshipSearchResultList.js
Lines 43 (patched)
<https://reviews.apache.org/r/74165/#comment313589>

    as this is used as a boolean, and with the default vaule, you can write it 
in initialize method.



dashboardv2/public/js/models/VRelationSearch.js
Lines 20 (patched)
<https://reviews.apache.org/r/74165/#comment313590>

    remove Globals, not used in this file



dashboardv2/public/js/models/VRelationSearch.js
Lines 23 (patched)
<https://reviews.apache.org/r/74165/#comment313591>

    remove Globals



dashboardv2/public/js/models/VRelationshipSearch.js
Lines 20 (patched)
<https://reviews.apache.org/r/74165/#comment313593>

    remove Globals



dashboardv2/public/js/models/VRelationshipSearch.js
Lines 23 (patched)
<https://reviews.apache.org/r/74165/#comment313594>

    remove Globals



dashboardv2/public/js/models/VRelationshipSearch.js
Lines 54 (patched)
<https://reviews.apache.org/r/74165/#comment313592>

    remove this commented lines



dashboardv2/public/js/templates/detail_page/RelationshipDetailPageLayoutView_tmpl.html
Lines 42 (patched)
<https://reviews.apache.org/r/74165/#comment313595>

    style="width: 58%" is the common style used in this file for all the 
panels, this can be written in a class, and you can use that class here instead 
of inline styling.
    
    While changing this value manually on each panel of this file, there can be 
a chances that we can miss some panel style, this can be avoided by using singe 
class.



dashboardv2/public/js/templates/detail_page/RelationshipDetailPageLayoutView_tmpl.html
Lines 73 (patched)
<https://reviews.apache.org/r/74165/#comment313596>

    refer above comment



dashboardv2/public/js/templates/detail_page/RelationshipDetailPageLayoutView_tmpl.html
Lines 97 (patched)
<https://reviews.apache.org/r/74165/#comment313597>

    refer above comment



dashboardv2/public/js/utils/CommonViewFunction.js
Line 200 (original), 201 (patched)
<https://reviews.apache.org/r/74165/#comment313598>

    ((_.isObject(inputOutputField) && !id) || (_.isObject(inputOutputField) && 
isRelationshipAttr))
    
    can be written as
    
    ((_.isObject(inputOutputField) && (!id || isRelationshipAttr))



dashboardv2/public/js/utils/CommonViewFunction.js
Lines 466 (patched)
<https://reviews.apache.org/r/74165/#comment313599>

    add 1 additional check for relationshipFilters.rules
    
    if(relationshipFilters.rules && relationshipFilters.rules.length ==1)? :



dashboardv2/public/js/utils/UrlLinks.js
Lines 198 (patched)
<https://reviews.apache.org/r/74165/#comment313600>

    this method should return something, here "if" should have "else" which can 
be used to return false or the required value, in case of searchtype is 
undefined.
    or
    if this method always has a searchtype value, then there is no need of if 
condition.



dashboardv2/public/js/views/detail_page/RelationshipDetailPageLayoutView.js
Lines 75 (patched)
<https://reviews.apache.org/r/74165/#comment313601>

    use "var" instead of "this", as this is not used publicly.



dashboardv2/public/js/views/detail_page/RelationshipDetailPageLayoutView.js
Lines 93 (patched)
<https://reviews.apache.org/r/74165/#comment313602>

    this.name use var name



dashboardv2/public/js/views/search/RelationSearchDetailLayoutView.js
Lines 46 (patched)
<https://reviews.apache.org/r/74165/#comment313603>

    _.pick() is not needed, as all the picked options are not used in this 
file, remove this line.



dashboardv2/public/js/views/search/RelationSearchLayoutView.js
Lines 72 (patched)
<https://reviews.apache.org/r/74165/#comment313604>

    add this relationshipFilters in Router.js, and remove from here.



dashboardv2/public/js/views/search/RelationSearchLayoutView.js
Lines 113 (patched)
<https://reviews.apache.org/r/74165/#comment313605>

    write in a variable _.extend() object, and return that variable.



dashboardv2/public/js/views/search/RelationSearchLayoutView.js
Lines 144 (patched)
<https://reviews.apache.org/r/74165/#comment313606>

    remove that, not used in this method



dashboardv2/public/js/views/search/RelationSearchLayoutView.js
Lines 188 (patched)
<https://reviews.apache.org/r/74165/#comment313607>

    remove that, not used in this method



dashboardv2/public/js/views/search/RelationSearchLayoutView.js
Lines 252 (patched)
<https://reviews.apache.org/r/74165/#comment313608>

    remove that,isTag,filtertype as not used in this method



dashboardv2/public/js/views/search/RelationSearchResultLayoutView.js
Lines 170 (patched)
<https://reviews.apache.org/r/74165/#comment313609>

    remove that variable



dashboardv2/public/js/views/search/RelationSearchResultLayoutView.js
Lines 205 (patched)
<https://reviews.apache.org/r/74165/#comment313610>

    write 
    
    value = {
    'query': null,
    'searchType': 'basic'
    };
    
    instead of keeping empty object.
    
    and remove the else part from line no.209



dashboardv2/public/js/views/search/RelationSearchResultLayoutView.js
Lines 206 (patched)
<https://reviews.apache.org/r/74165/#comment313612>

    remove that variable



dashboardv2/public/js/views/search/RelationSearchResultLayoutView.js
Lines 209 (patched)
<https://reviews.apache.org/r/74165/#comment313611>

    remove this else part, please refer line no. 205 comment



dashboardv2/public/js/views/search/SearchLayoutView.js
Lines 85 (patched)
<https://reviews.apache.org/r/74165/#comment313613>

    this entire if else can be written as:
    
    var redirectionUrl = '#!/search';
    if(this.isRelationSearch)
    url=  '#!/relationship';
    
    Utils.setUrl({
    url: redirectionUrl,
    mergeBrowserUrl: false,
    trigger: true,
    updateTabState: true
    });
    
    we can avoid duplicate code.


- Prasad Pawar


On Oct. 12, 2022, 11:20 a.m., Farhan Khan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74165/
> -----------------------------------------------------------
> 
> (Updated Oct. 12, 2022, 11:20 a.m.)
> 
> 
> Review request for atlas and Prasad Pawar.
> 
> 
> Bugs: ATLAS-4686
>     https://issues.apache.org/jira/browse/ATLAS-4686
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> Current Working :
> 
> Atlas is entirely entity-based – it is not possible to search for 
> relationships.
> 
> 
> Diffs
> -----
> 
>   dashboardv2/public/css/scss/override.scss 7a14abf6d 
>   dashboardv2/public/css/scss/tab.scss c164b4744 
>   dashboardv2/public/css/scss/tag.scss c7bfb7e72 
>   dashboardv2/public/js/collection/VRelationshipSearchList.js PRE-CREATION 
>   dashboardv2/public/js/collection/VRelationshipSearchResultList.js 
> PRE-CREATION 
>   dashboardv2/public/js/main.js 0994c1b02 
>   dashboardv2/public/js/models/VRelationSearch.js PRE-CREATION 
>   dashboardv2/public/js/models/VRelationshipSearch.js PRE-CREATION 
>   dashboardv2/public/js/router/Router.js b3b0b961e 
>   
> dashboardv2/public/js/templates/detail_page/RelationshipDetailPageLayoutView_tmpl.html
>  PRE-CREATION 
>   
> dashboardv2/public/js/templates/entity/EntityDetailTableLayoutView_tmpl.html 
> 3fe0e95e5 
>   
> dashboardv2/public/js/templates/search/RelationSearchDetailLayoutView_tmpl.html
>  PRE-CREATION 
>   
> dashboardv2/public/js/templates/search/RelationSearchResultLayoutView_tmpl.html
>  PRE-CREATION 
>   dashboardv2/public/js/templates/search/RelationshipSearch_tmpl.html 
> PRE-CREATION 
>   dashboardv2/public/js/templates/search/SearchLayoutView_tmpl.html 3bc3121f4 
>   dashboardv2/public/js/utils/CommonViewFunction.js 4ce47a13d 
>   dashboardv2/public/js/utils/Enums.js ee80313c2 
>   dashboardv2/public/js/utils/Globals.js d2ce1a90f 
>   dashboardv2/public/js/utils/UrlLinks.js 243b459be 
>   dashboardv2/public/js/utils/Utils.js eb0588903 
>   dashboardv2/public/js/views/detail_page/DetailPageLayoutView.js bb629b4e4 
>   dashboardv2/public/js/views/detail_page/RelationshipDetailPageLayoutView.js 
> PRE-CREATION 
>   dashboardv2/public/js/views/entity/EntityDetailTableLayoutView.js 5eee4d711 
>   dashboardv2/public/js/views/search/QueryBuilderView.js 983f8b759 
>   dashboardv2/public/js/views/search/RelationSearchDetailLayoutView.js 
> PRE-CREATION 
>   dashboardv2/public/js/views/search/RelationSearchLayoutView.js PRE-CREATION 
>   dashboardv2/public/js/views/search/RelationSearchResultLayoutView.js 
> PRE-CREATION 
>   dashboardv2/public/js/views/search/SearchLayoutView.js 675a521ce 
>   dashboardv2/public/js/views/search/SearchQueryView.js 69a35a54b 
>   dashboardv2/public/js/views/search/save/SaveModalLayoutView.js 350758874 
>   dashboardv2/public/js/views/search/save/SaveSearchItemView.js a86eff4e0 
>   dashboardv2/public/js/views/search/save/SaveSearchView.js 436d83db7 
>   dashboardv2/public/js/views/site/Header.js ffb582ed4 
>   dashboardv2/public/js/views/site/SideNavLayoutView.js e0f717a24 
>   dashboardv3/public/css/scss/leftsidebar.scss 68a6f79a1 
>   dashboardv3/public/js/collection/VRelationshipSearchList.js PRE-CREATION 
>   dashboardv3/public/js/collection/VRelationshipSearchResultList.js 
> PRE-CREATION 
>   dashboardv3/public/js/main.js b1aaec26b 
>   dashboardv3/public/js/models/VRelationSearch.js PRE-CREATION 
>   dashboardv3/public/js/models/VRelationshipSearch.js PRE-CREATION 
>   dashboardv3/public/js/router/Router.js 8e8cf1557 
>   
> dashboardv3/public/js/templates/detail_page/RelationshipDetailPageLayoutView_tmpl.html
>  PRE-CREATION 
>   
> dashboardv3/public/js/templates/entity/EntityDetailTableLayoutView_tmpl.html 
> 3fe0e95e5 
>   
> dashboardv3/public/js/templates/search/RelationSearchResultLayoutView_tmpl.html
>  PRE-CREATION 
>   dashboardv3/public/js/templates/search/SearchDefaultLayoutView_tmpl.html 
> 7a5a322c7 
>   
> dashboardv3/public/js/templates/search/SearchFilterBrowseLayoutView_tmpl.html 
> 9ca345539 
>   dashboardv3/public/js/templates/search/SearchResultLayoutView_tmpl.html 
> b16a4d28a 
>   
> dashboardv3/public/js/templates/search/tree/RelationshipLayoutView_tmpl.html 
> PRE-CREATION 
>   dashboardv3/public/js/utils/CommonViewFunction.js 7d8f03d2e 
>   dashboardv3/public/js/utils/Enums.js ee80313c2 
>   dashboardv3/public/js/utils/Globals.js 9a51c4dc4 
>   dashboardv3/public/js/utils/UrlLinks.js b9a9c8174 
>   dashboardv3/public/js/utils/Utils.js 0d8722fa9 
>   dashboardv3/public/js/views/detail_page/DetailPageLayoutView.js 471be3bd7 
>   dashboardv3/public/js/views/detail_page/RelationshipDetailPageLayoutView.js 
> PRE-CREATION 
>   dashboardv3/public/js/views/entity/EntityDetailTableLayoutView.js 5eee4d711 
>   dashboardv3/public/js/views/search/QueryBuilderView.js 3b4a95317 
>   dashboardv3/public/js/views/search/RelationSearchResultLayoutView.js 
> PRE-CREATION 
>   dashboardv3/public/js/views/search/SearchDefaultLayoutView.js b65f753d0 
>   dashboardv3/public/js/views/search/SearchFilterBrowseLayoutView.js 
> 86916f45d 
>   dashboardv3/public/js/views/search/SearchQueryView.js 1f658e436 
>   dashboardv3/public/js/views/search/save/SaveModalLayoutView.js 6e6e64990 
>   dashboardv3/public/js/views/search/tree/ClassificationTreeLayoutView.js 
> 1883dfbd7 
>   dashboardv3/public/js/views/search/tree/CustomFilterTreeLayoutView.js 
> eadd64180 
>   dashboardv3/public/js/views/search/tree/RelationshipSearchTreeLayoutView.js 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/74165/diff/1/
> 
> 
> Testing
> -------
> 
> Manually tested for the newly added Relationship Search on both UI.
> 
> 
> Thanks,
> 
> Farhan Khan
> 
>

Reply via email to