Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ui): add exec check to avoid API calls #16168

Merged
merged 6 commits into from
Nov 30, 2023

Conversation

ashutosh16
Copy link
Contributor

@ashutosh16 ashutosh16 commented Oct 30, 2023

fixes #16165

Along with this PR, fixing UI bug on the resource list when pod group is enabled and total pod count is grater than 15
UI displays Parent Node Ref details which shouldn't be displayed on this list view. Parent Ref details should only be displayed when the user clicks on the pod group on the tree view
Screenshot 2023-10-30 at 7 00 56 PM

@ashutosh16 ashutosh16 requested a review from a team as a code owner October 30, 2023 21:56
@ashutosh16 ashutosh16 marked this pull request as draft October 31, 2023 01:28
@ashutosh16 ashutosh16 changed the title bug: add exec check to avoid API calls fix: add exec check to avoid API calls Oct 31, 2023
@ashutosh16 ashutosh16 force-pushed the bug/mutliple-can-I-exec branch 2 times, most recently from 958ef84 to 732db13 Compare October 31, 2023 03:08
@ashutosh16
Copy link
Contributor Author

ashutosh16 commented Oct 31, 2023

Fix - 1
no api call to [can-i/exec/create/test/argo-analysis](http://localhost:4000/api/v1/account/can-i/exec/create/test/argo-analysis)

Screenshot 2023-10-30 at 8 11 21 PM

Fix -2

Screen.Recording.2023-10-30.at.8.08.56.PM.mov

@ashutosh16 ashutosh16 force-pushed the bug/mutliple-can-I-exec branch 2 times, most recently from f5b4023 to cb1c75a Compare October 31, 2023 20:00
Copy link

codecov bot commented Oct 31, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (4875b02) 49.53% compared to head (d253053) 49.53%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #16168   +/-   ##
=======================================
  Coverage   49.53%   49.53%           
=======================================
  Files         269      269           
  Lines       47321    47321           
=======================================
  Hits        23441    23441           
  Misses      21576    21576           
  Partials     2304     2304           

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

@ashutosh16 ashutosh16 marked this pull request as ready for review November 1, 2023 18:09
@ashutosh16
Copy link
Contributor Author

@ishitasequeira Would it be possible for you to test my changes. There are two issues that this PR addresses.

  • Check settings.execEnabled to determine whether exec is enabled, and only make API queries if exec is enabled.
  • Determine the location of the UI - resource list or tree view. Only display the UI component when the user is viewing the tree or network

@ishitasequeira
Copy link
Member

@ishitasequeira Would it be possible for you to test my changes. There are two issues that this PR addresses.

  • Check settings.execEnabled to determine whether exec is enabled, and only make API queries if exec is enabled.
  • Determine the location of the UI - resource list or tree view. Only display the UI component when the user is viewing the tree or network

Sure, I will test it out this week.

@chengfang
Copy link
Contributor

@ashutosh16 can you include fix(ui) in the title since this PR is all about ui changes. Thanks.

@ashutosh16 ashutosh16 changed the title fix: add exec check to avoid API calls fix(ui): add exec check to avoid API calls Nov 2, 2023
Copy link
Member

@ishitasequeira ishitasequeira left a comment

Choose a reason for hiding this comment

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

Thanks @ashutosh16, I tested the change and the changes look good to me.

@ashutosh16
Copy link
Contributor Author

@keithchong /@ishitasequeira Would you be able to merge the PR?

Copy link
Member

@ishitasequeira ishitasequeira left a comment

Choose a reason for hiding this comment

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

Thanks @ashutosh16 for fixing the comments.

@ishitasequeira ishitasequeira merged commit 9e92f55 into argoproj:master Nov 30, 2023
25 checks passed
vladfr pushed a commit to vladfr/argo-cd that referenced this pull request Dec 13, 2023
* bug: add parent ref node info on resource list

Signed-off-by: ashutosh16 <[email protected]>

* bug: add parent ref node info on resource list

Signed-off-by: ashutosh16 <[email protected]>

* bug: add parent ref node info on resource list

Signed-off-by: ashutosh16 <[email protected]>

* bug: add parent ref node info on resource list

Signed-off-by: ashutosh16 <[email protected]>

* bug: add parent ref node info on resource list

Signed-off-by: ashutosh16 <[email protected]>

* bug: add parent ref node info on resource list

Signed-off-by: ashutosh16 <[email protected]>

---------

Signed-off-by: ashutosh16 <[email protected]>
tesla59 pushed a commit to tesla59/argo-cd that referenced this pull request Dec 16, 2023
* bug: add parent ref node info on resource list

Signed-off-by: ashutosh16 <[email protected]>

* bug: add parent ref node info on resource list

Signed-off-by: ashutosh16 <[email protected]>

* bug: add parent ref node info on resource list

Signed-off-by: ashutosh16 <[email protected]>

* bug: add parent ref node info on resource list

Signed-off-by: ashutosh16 <[email protected]>

* bug: add parent ref node info on resource list

Signed-off-by: ashutosh16 <[email protected]>

* bug: add parent ref node info on resource list

Signed-off-by: ashutosh16 <[email protected]>

---------

Signed-off-by: ashutosh16 <[email protected]>
JulienFuix pushed a commit to JulienFuix/argo-cd that referenced this pull request Feb 6, 2024
* bug: add parent ref node info on resource list

Signed-off-by: ashutosh16 <[email protected]>

* bug: add parent ref node info on resource list

Signed-off-by: ashutosh16 <[email protected]>

* bug: add parent ref node info on resource list

Signed-off-by: ashutosh16 <[email protected]>

* bug: add parent ref node info on resource list

Signed-off-by: ashutosh16 <[email protected]>

* bug: add parent ref node info on resource list

Signed-off-by: ashutosh16 <[email protected]>

* bug: add parent ref node info on resource list

Signed-off-by: ashutosh16 <[email protected]>

---------

Signed-off-by: ashutosh16 <[email protected]>
lyda pushed a commit to lyda/argo-cd that referenced this pull request Mar 28, 2024
* bug: add parent ref node info on resource list

Signed-off-by: ashutosh16 <[email protected]>

* bug: add parent ref node info on resource list

Signed-off-by: ashutosh16 <[email protected]>

* bug: add parent ref node info on resource list

Signed-off-by: ashutosh16 <[email protected]>

* bug: add parent ref node info on resource list

Signed-off-by: ashutosh16 <[email protected]>

* bug: add parent ref node info on resource list

Signed-off-by: ashutosh16 <[email protected]>

* bug: add parent ref node info on resource list

Signed-off-by: ashutosh16 <[email protected]>

---------

Signed-off-by: ashutosh16 <[email protected]>
Signed-off-by: Kevin Lyda <[email protected]>
Hariharasuthan99 pushed a commit to AmadeusITGroup/argo-cd that referenced this pull request Jun 16, 2024
* bug: add parent ref node info on resource list

Signed-off-by: ashutosh16 <[email protected]>

* bug: add parent ref node info on resource list

Signed-off-by: ashutosh16 <[email protected]>

* bug: add parent ref node info on resource list

Signed-off-by: ashutosh16 <[email protected]>

* bug: add parent ref node info on resource list

Signed-off-by: ashutosh16 <[email protected]>

* bug: add parent ref node info on resource list

Signed-off-by: ashutosh16 <[email protected]>

* bug: add parent ref node info on resource list

Signed-off-by: ashutosh16 <[email protected]>

---------

Signed-off-by: ashutosh16 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug(ui): Add check execEnabled to avoid extra API call
4 participants