-
Notifications
You must be signed in to change notification settings - Fork 114
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
Fixed the parallel API calling for the org details tabs(Cookbooks, Roles and Org edit tabs) #3589
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haven't gone through all of this, just added something that caught my eye. Sorry for the half-review 😅
components/automate-ui/src/app/modules/infra-proxy/cookbooks/cookbooks.component.spec.ts
Outdated
Show resolved
Hide resolved
components/automate-ui/src/app/modules/infra-proxy/cookbooks/cookbooks.component.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for addressing my other comments! I'm afraid I have another two small ones to share 😉
components/automate-ui/src/app/modules/infra-proxy/cookbooks/cookbooks.component.spec.ts
Show resolved
Hide resolved
components/automate-ui/src/app/modules/infra-proxy/infra-roles/infra-roles.component.html
Outdated
Show resolved
Hide resolved
d4dda6d
to
bafac6c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found a couple duplicates - but overall is looking pretty good 👍
I wasn't able to totally test this out, when I get to an org detail page I have an error, but its because I don't have a valid org setup to check. I used infra_service_load_sample_data -N 10 -S infra-server -O infra-org
to set it up - do you happen to have the curl call for 4th coffee that I could use?
components/automate-ui/src/app/modules/infra-proxy/cookbooks/cookbooks.component.spec.ts
Outdated
Show resolved
Hide resolved
components/automate-ui/src/app/modules/infra-proxy/org-edit/org-edit.component.spec.ts
Outdated
Show resolved
Hide resolved
bafac6c
to
70c881e
Compare
@SEAjamieD you'll find the curl call you need to test this out pinned in #automate-infra-views slack -- it's the (currently) 2nd item pinned, from mar 6th |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public serverId; | ||
public OrgId; | ||
public updateOrgForm: FormGroup; | ||
public tabValue: OrgTabName = 'cookbooks'; | ||
public orgId; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One of these isn't yours, but would be cool if you could add types for these while you're in here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a few more comments to address otherwise I think this is looking really good 🎉
70c881e
to
7127933
Compare
Signed-off-by: vinay033 <[email protected]>
Signed-off-by: vinay033 <[email protected]>
Signed-off-by: vinay033 <[email protected]>
Signed-off-by: vinay033 <[email protected]>
Signed-off-by: vinay033 <[email protected]>
Signed-off-by: vinay033 <[email protected]>
Signed-off-by: vinay033 <[email protected]>
Signed-off-by: vinay033 <[email protected]>
Signed-off-by: vinay033 <[email protected]>
Signed-off-by: vinay033 <[email protected]>
20a800b
to
eb39583
Compare
Signed-off-by: vinay033 [email protected]
🔩 Description: What code changed, and why?
Org details page has different tabs, when we click to a particular tab then all API's calling parallel, on this page we are going to add more tabs so we have to manage like if we click to particular tab then only related API will calling.
I have added changes to separate the API calls, now if we click to particular tab then only specific API is called.
⛓️ Related Resources
#3410
👍 Definition of Done
we have separated the API calls now only specific tabs related API is calling when we clicking to a particular tab.
👟 How to Build and Test the Change
✅ Checklist
📷 Screenshots, if applicable