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: skip admin queries when consolidating REST APIs #7142

Merged
merged 1 commit into from
Apr 20, 2021
Merged

fix: skip admin queries when consolidating REST APIs #7142

merged 1 commit into from
Apr 20, 2021

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Apr 20, 2021

Description of changes

This commit skips over the admin queries REST API when consolidating the user's REST APIs.

Issue #, if available

Refs: #2084

Description of how you validated changes

Ran several failing E2E tests.

Checklist

  • PR description included
  • yarn test passes

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@cjihrig cjihrig requested a review from a team as a code owner April 20, 2021 20:54
@codecov-commenter
Copy link

Codecov Report

Merging #7142 (849ddc7) into master (f259e5a) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7142      +/-   ##
==========================================
- Coverage   51.54%   51.53%   -0.01%     
==========================================
  Files         486      486              
  Lines       25051    25053       +2     
  Branches     4913     4915       +2     
==========================================
  Hits        12912    12912              
- Misses      11187    11189       +2     
  Partials      952      952              
Impacted Files Coverage Δ
...dformation/src/utils/consolidate-apigw-policies.ts 13.22% <0.00%> (-0.23%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f259e5a...849ddc7. Read the comment docs.

@@ -231,7 +235,7 @@ export function loadApiWithPrivacyParams(context: $TSContext, name: string, reso
}

function updateExistingApiCfn(context: $TSContext, api: $TSObject): void {
const { resourceName } = api.params;
const resourceName = api.resourceName || api.params.resourceName;
Copy link
Contributor

Choose a reason for hiding this comment

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

should we have any defensive logic here to early return if resourceName is undefined?

@edwardfoyle edwardfoyle merged commit c8069bd into aws-amplify:master Apr 20, 2021
@cjihrig cjihrig deleted the admin branch April 20, 2021 23:26
@github-actions github-actions bot added the referenced-in-release Issues referenced in a published release changelog label Apr 27, 2021
@github-actions
Copy link

👋 Hi, this pull request was referenced in the v4.50.0 release!

Check out the release notes here https://github.com/aws-amplify/amplify-cli/releases/tag/v4.50.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
referenced-in-release Issues referenced in a published release changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants