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

UIE-204 Narrow Ajax Usage pt22 #5229

Merged
merged 3 commits into from
Jan 24, 2025
Merged

Conversation

msilva-broad
Copy link
Contributor

@msilva-broad msilva-broad commented Jan 23, 2025

  • narrowed Ajax() usage within src/data-catalog area to call Ajax().SubAreaX directly.
  • this is the last usage outside of src/libs/ajax, which will be cleaned up next.

Jira Ticket: https://broadworkbench.atlassian.net/browse/[Ticket #]

Summary of changes:

What

Why

  • improve code maintainability

Testing strategy

@msilva-broad msilva-broad force-pushed the msilva/UIE-204_Narrow-ajax-usage_pt22 branch from 32f6446 to af2257e Compare January 23, 2025 16:06
@msilva-broad msilva-broad marked this pull request as ready for review January 23, 2025 16:42
@msilva-broad msilva-broad requested a review from a team as a code owner January 23, 2025 16:42
- narrowed Ajax() usage within src/data-catalog area to call Ajax().SubAreaX directly.
- this is the last usage outside of src/libs/ajax, which will be cleaned up next.
- minor import cleanup
@msilva-broad msilva-broad force-pushed the msilva/UIE-204_Narrow-ajax-usage_pt22 branch from af2257e to eb590b3 Compare January 23, 2025 19:08
Copy link
Member

@pshapiro4broad pshapiro4broad left a comment

Choose a reason for hiding this comment

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

Looks OK. I think you could also remove the Catalog member from Ajax with this PR.

@@ -4,7 +4,7 @@ import { div, h } from 'react-hyperscript-helpers';
import { ButtonPrimary, spinnerOverlay } from 'src/components/common';
import FooterWrapper from 'src/components/FooterWrapper';
import { TopBar } from 'src/components/TopBar';
import { Ajax } from 'src/libs/ajax';
import { Catalog } from 'src/libs/ajax/Catalog';
Copy link
Member

@pshapiro4broad pshapiro4broad Jan 24, 2025

Choose a reason for hiding this comment

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

Since Catalog is also imported from below, sonar flags this as a case where imports can be merged. Can you check and address sonar warnings added with this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah, missed that. will do

@msilva-broad
Copy link
Contributor Author

Looks OK. I think you could also remove the Catalog member from Ajax with this PR.

removing ajax members (and related cleanup) will be the final PR after I remove some last references from within src/libs/ajax (mostly providers). I need to confirm some back-door AoU usage will be ok... a very small set will remain for some end2end test utility usage (which I am not pushing to remove yet).... but that can be from a demoted initAjaxForTesting function that is called from app startup....

Copy link
Contributor

@fboulnois fboulnois left a comment

Choose a reason for hiding this comment

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

Looks good, thank you 👍

- minor import cleanup
@msilva-broad msilva-broad added this pull request to the merge queue Jan 24, 2025
Merged via the queue into dev with commit 7679b0d Jan 24, 2025
11 checks passed
@msilva-broad msilva-broad deleted the msilva/UIE-204_Narrow-ajax-usage_pt22 branch January 24, 2025 19:37
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.

4 participants