fix: prevent removal of analytics and auth when being depended on

[edwardfoyle]

Description of changes

Adds a check in the remove analytics handler to throw if notifications exists (because it depends on analytics) and likewise adds a check in auth remove to throw if analytics exists (because analytics depends on auth). In both cases an error message is printed telling the customer to remove the upstream dependency first, then retry the removal

Issue #, if available

Description of how you validated changes

Manually tested and added a unit tests.

Checklist

• PR description included
• yarn test passes
• Tests are changed or added

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

[codecov-commenter]

Codecov Report

Merging #11210 (f139150) into dev (52f5787) will increase coverage by 0.02%.
The diff coverage is 95.83%.

@@            Coverage Diff             @@
##              dev   #11210      +/-   ##
==========================================
+ Coverage   49.36%   49.39%   +0.02%     
==========================================
  Files         678      680       +2     
  Lines       32692    32723      +31     
  Branches     6674     6680       +6     
==========================================
+ Hits        16138    16163      +25     
- Misses      15074    15080       +6     
  Partials     1480     1480              

Impacted Files	Coverage Δ
...y-analytics/src/plugin-client-api-notifications.ts	41.66% <0.00%> (ø)
...s/amplify-cli-core/src/errors/amplify-exception.ts	65.21% <ø> (ø)
...ategory-analytics/src/commands/analytics/remove.ts	100.00% <100.00%> (ø)
.../amplify-category-auth/src/commands/auth/remove.ts	87.87% <100.00%> (+2.69%)	⬆️
...li/src/domain/amplify-usageData/getUsageDataUrl.ts	100.00% <0.00%> (+12.50%)	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[sobolk]

This might be long term.

1. Is there a way to declare dependencies at CFN template level for this?
2. If not, should we develop a mechanism/pattern where we can declare dependencies between categories in less coupling way ?

[edwardfoyle]

Re your long term concerns @sobolk:

1. I think we should abstract resource dependencies above the level of having to parse CFN to determine the connections
2. Yes we definitely need a project-wide implementation for keeping track of resource dependencies and failing on cycles / missing dependencies in a single place. But that's much bigger than this point fix

[sobolk]

Re your long term concerns @sobolk:

1. I think we should abstract resource dependencies above the level of having to parse CFN to determine the connections
2. Yes we definitely need a project-wide implementation for keeping track of resource dependencies and failing on cycles / missing dependencies in a single place. But that's much bigger than this point fix

let's log this work in backlog then if we haven't already. we should set a threshold when adhoc mechnisms like that prompt the refactoring if we want to avoid complexity explosion.

[awsluja]

why did analytics get removed from here? Does Analytics not depend on Auth?

[edwardfoyle]

now that there's an upstream check to fail if analytics exists, we don't need to warn about it here

[sobolk]

I was about to ask a question here as well. Why do we have two mechanism for similar problem but they just differ by severity - i.e. one throws the other warns . (perhaps this should go to the long term follow up we should do here at some point).

[edwardfoyle]

It's a good question that I don't know the answer to. I was just trying to keep the behavior change as localized as possible for this PR

[awsluja]

Ok, I see what's going on now; not a blocker IMO