Added displaynetwork option in filters for listnetwork only for admin

[harikrishna-patnala]

Description

This PR addresses the issue #9386

With this PR, admin will now have the new option in filters section in list networks page. If a network is edited by setting "Display Network" to false, it will still redirect to 404 page, but then have the below option to list those kind of networks.

Types of changes

• Breaking change (fix or feature that would cause existing functionality to change)
• New feature (non-breaking change which adds functionality)
• Bug fix (non-breaking change which fixes an issue)
• Enhancement (improves an existing feature and functionality)
• Cleanup (Code refactoring and cleanup, that may add test cases)
• build/CI
• test (unit or integration test code)

Bug Severity

• BLOCKER
• Critical
• Major
• Minor
• Trivial

Screenshots (if appropriate):

How Has This Been Tested?

1. Edit a network (test) and set "Display network" to false, then list networks
   

2. Update filter option, set "Display network" to false
   

3. click Search, Networks to which display networks set to false will be shown
   

How did you try to break this feature and the system with this change?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 15.13%. Comparing base (9967bb3) to head (f4d3042).
Report is 12 commits behind head on 4.19.

Additional details and impacted files

@@            Coverage Diff            @@
##               4.19   #10209   +/-   ##
=========================================
  Coverage     15.13%   15.13%           
- Complexity    11274    11279    +5     
=========================================
  Files          5408     5408           
  Lines        473974   474016   +42     
  Branches      57813    57824   +11     
=========================================
+ Hits          71730    71746   +16     
- Misses       394227   394248   +21     
- Partials       8017     8022    +5     

Flag	Coverage Δ
uitests	4.29% <ø> (-0.01%)	⬇️
unittests	15.85% <ø> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[DaanHoogland]

clgtm

[DaanHoogland]

@blueorangutan ui

[blueorangutan]

@DaanHoogland a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

[blueorangutan]

UI build: ✔️
Live QA URL: https://qa.cloudstack.cloud/simulator/pr/10209 (QA-JID-512)

[DaanHoogland]

tested ok in qa!
@harikrishna-patnala this may be a bit counterintuitive for users, don't you think? The display in the list being possible but the details not. Maybe we can find a good spot to document/explain this?

[harikrishna-patnala]

tested ok in qa! @harikrishna-patnala this may be a bit counterintuitive for users, don't you think? The display in the list being possible but the details not. Maybe we can find a good spot to document/explain this?

I suppose users you mean the admins! The display is possible only when they select "Display Network" to false, so I think it is intuitive. But what I just realized is clicking on that network leads to a 404 error. I'm thinking to extend this PR to include that as well. Any objections on that ?

[harikrishna-patnala]

@blueorangutan ui

[harikrishna-patnala]

@DaanHoogland can you please check again, added fix for that too.

[DaanHoogland]

@blueorangutan ui

[blueorangutan]

@DaanHoogland a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

[blueorangutan]

UI build: ✔️
Live QA URL: https://qa.cloudstack.cloud/simulator/pr/10209 (QA-JID-516)

[shwstppr]

Code LGTM but for such networks opened from other than listview it would still return 404. Others can advise. Maybe in future we improve this API parameter which somehow allows list a network irrespective of network's displaynetwork value for admins

[DaanHoogland]

Code LGTM but for such networks opened from other than listview it would still return 404. Others can advise. Maybe in future we improve this API parameter which somehow allows list a network irrespective of network's displaynetwork value for admins

I had the same finding @shwstppr but agree this is another issue (UI polish). At least now the network can be viewed in the UI at all.

[DaanHoogland]

@harikrishna-patnala do you want to polish this more now or merge and postpone further enhancements?

[harikrishna-patnala]

Did some code investigation and I agreed with @shwstppr this has to be improved from API param, so I think we are good with the existing PR changes @DaanHoogland if you agree let's merge this.

[DaanHoogland]

ok, I think the remaining 404 is actually valid and how to deal with it requires functional agreement.

• one take would be to revert to the list
• another one to be cleaver about the display parameter

I actually like neither ;)
merging