Skip to content

UI: Removed redundant IP Address Column#8275

Merged
weizhouapache merged 1 commit intoapache:4.18from
OlegChuev:fix/remove-redundant-ip-address-column
Dec 1, 2023
Merged

UI: Removed redundant IP Address Column#8275
weizhouapache merged 1 commit intoapache:4.18from
OlegChuev:fix/remove-redundant-ip-address-column

Conversation

@OlegChuev
Copy link
Contributor

@OlegChuev OlegChuev commented Nov 28, 2023 •
edited
Loading

Description

This PR removes redundant IP Address column

Link to the issue: #8273

Types of changes

  • Enhancement (improves an existing feature and functionality)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Minor

Bug Severity

  • Trivial

Screenshots (if appropriate):

Before fix:
Screenshot 2023-11-27 at 19 01 34

After fix:
Screenshot 2023-11-28 at 11 34 52

How Has This Been Tested?

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

@OlegChuev OlegChuev mentioned this pull request Nov 28, 2023
Closed
@DaanHoogland
Copy link
Contributor

DaanHoogland commented Nov 28, 2023

code looks good, but I am not sure if an operator wouldn´t want to see the ip during configuration of the pf rules. If it is not available though , ... It seems strange and may be a bug.

@codecov
Copy link

codecov bot commented Nov 28, 2023 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (88ff77d) 12.93% compared to head (a67d801) 27.05%.
Report is 537 commits behind head on main.

Additional details and impacted files 
@@              Coverage Diff              @@
##               main    #8275       +/-   ##
=============================================
+ Coverage     12.93%   27.05%   +14.12%     
- Complexity     8944    28472    +19528     
=============================================
  Files          2715     5193     +2478     
  Lines        256107   366322   +110215     
  Branches      39938    53558    +13620     
=============================================
+ Hits          33139    99125    +65986     
- Misses       218808   253346    +34538     
- Partials       4160    13851     +9691
Flag Coverage Δ
simulator-marvin-tests 22.35% <ø> (?)
uitests 4.49% <ø> (?)
unit-tests 14.81% <ø> (?)

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.

weizhouapache
weizhouapache approved these changes Nov 28, 2023
View reviewed changes
Copy link
Member

@weizhouapache weizhouapache left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code lgtm

@weizhouapache
Copy link
Member

weizhouapache commented Nov 28, 2023

code looks good, but I am not sure if an operator wouldn´t want to see the ip during configuration of the pf rules. If it is not available though , ... It seems strange and may be a bug.

the IP is displayed when a VM is choosed.
now the view is almost same as the view when adding load balancing rules (IPs are not displayed in the list)

@DaanHoogland
Copy link
Contributor

DaanHoogland commented Nov 28, 2023

@blueorangutan ui

@blueorangutan
Copy link

blueorangutan commented Nov 28, 2023

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

@blueorangutan
Copy link

blueorangutan commented Nov 28, 2023

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

DaanHoogland
DaanHoogland approved these changes Nov 28, 2023
View reviewed changes
Copy link
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checked in qa
clgtm

@DaanHoogland
Copy link
Contributor

DaanHoogland commented Nov 28, 2023

@weizhouapache @OlegChuev should this go on the 4.18 branch?

@OlegChuev
Copy link
Contributor Author

OlegChuev commented Nov 29, 2023

should this go on the 4.18 branch?

yep, sorry, I missed it. I'll change the target branch

@OlegChuev OlegChuev changed the base branch from main to 4.18 November 29, 2023 10:46
@DaanHoogland
Copy link
Contributor

DaanHoogland commented Nov 29, 2023

@blueorangutan ui

@DaanHoogland DaanHoogland added this to the 4.18.2.0 milestone Nov 29, 2023
@blueorangutan
Copy link

blueorangutan commented Nov 29, 2023

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

@weizhouapache
Copy link
Member

weizhouapache commented Dec 1, 2023

merging based on 2 approvals and manual testing result

@weizhouapache weizhouapache merged commit 7d6dc41 into apache:4.18 Dec 1, 2023
@OlegChuev OlegChuev deleted the fix/remove-redundant-ip-address-column branch December 1, 2023 09:00
DaanHoogland added a commit that referenced this pull request Dec 4, 2023
@DaanHoogland
Merge release branch 4.18 to main
d3cad42 
* 4.18:
  server: Initial new vpnuser state (#8268)
  UI: Removed redundant IP Address Column when create Port forwarding rules (#8275)
  UI: Removed ICMP input fields for protocol number from ACL List rules modal (#8253)
  server: check if there are active nics before network GC (#8204)
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Dec 6, 2023
@OlegChuev @dhslove
UI: Removed redundant IP Address Column when create Port forwarding r…
4908076 
…ules (apache#8275)
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Dec 6, 2023
@DaanHoogland @dhslove
Merge release branch 4.18 to main
118ae64 
* 4.18:
  server: Initial new vpnuser state (apache#8268)
  UI: Removed redundant IP Address Column when create Port forwarding rules (apache#8275)
  UI: Removed ICMP input fields for protocol number from ACL List rules modal (apache#8253)
  server: check if there are active nics before network GC (apache#8204)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DaanHoogland DaanHoogland DaanHoogland approved these changes

@weizhouapache weizhouapache weizhouapache approved these changes

Assignees

No one assigned

Labels

component:UI

Projects

None yet

Milestone

4.18.2.0

Development

Successfully merging this pull request may close these issues.

5 participants

@OlegChuev @DaanHoogland @weizhouapache @blueorangutan @OlegChuev-MD