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

WIP use product code to lookup product install when displayName returns null #11479

Merged
merged 3 commits into from
Feb 11, 2021

Conversation

mjkkirschner
Copy link
Member

Purpose

I found that even though we started cacheing the product keys during asm product lookup - we were not actually using them to look in the registry to find the matching product - we were only using display name.

certain products no longer use their display name, and instead use their product code.

The following PR first tries to lookup the product in the cached registry data using the product name, then it uses the product key if the former returns null.

After this PR DynamoSandbox on my machine finds Revit 2022 and Revit 2021 as products with asm.

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated

@mjkkirschner mjkkirschner requested a review from pinzart February 8, 2021 22:01
@mjkkirschner mjkkirschner changed the title wip use prod code when name returns null WIP use product code to lookup product install when displayName returns null Feb 8, 2021
Copy link
Contributor

@pinzart90 pinzart90 left a comment

Choose a reason for hiding this comment

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

LGTM
Unit test for this case?

add friend assembly for moq
add test
fix broken tests
add fallback for interface
@mjkkirschner mjkkirschner added PTAL Please Take A Look 👀 and removed WIP labels Feb 9, 2021
@mjkkirschner
Copy link
Member Author

@pinzart90 please take another look - I think I've added a sufficient test and fixed the other tests that this broke (their mocks needed the new method added)

I also added a fallback if someone passes an interface type to this method.

@mjkkirschner
Copy link
Member Author

@pinzart90 I think i'd like to wait on merging this until the build is back up.

@mjkkirschner mjkkirschner merged commit 8831ecd into DynamoDS:master Feb 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PTAL Please Take A Look 👀
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants