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.

Sign up for GitHub

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

Jump to bottom

Scrub code for uses of navigations that don't cover skip navigations #22883

Closed
13 tasks done
ajcvickers opened this issue Oct 4, 2020 · 2 comments
Closed
13 tasks done

Scrub code for uses of navigations that don't cover skip navigations #22883

ajcvickers opened this issue Oct 4, 2020 · 2 comments
Labels
area-global closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. Servicing-approved type-bug
Milestone
5.0.0

Comments

@ajcvickers
Copy link
Member

ajcvickers commented Oct 4, 2020 •
edited
Loading

ajcvickers added a commit that referenced this issue Oct 4, 2020
@ajcvickers
[5.0] Fix places where skip navigations are missed
7f1a852 
Fixes #22882

**Description**

Skip navigations (i.e. many-to-many navigations; new for 5.0) are not being returned as members, navigations, or collections from an an API used to access them. In addition, scrubbing the code found two other issues with the same pattern, plus a few that need more investigation. This PR contains fixes and tests for the three clear issues. I have filed #22883 to follow up on the others.

**Customer Impact**

* Navigations for a major new feature are not being returned from an API that should return them.
* Attaching a graph asynchronously may miss navigations
* Skip navigations are missed as notification properties

**How found**

Customer reported on RC1.

**Test coverage**

This PR adds tests for the affected cases, including others found from scrubbing the code. #22883 tracks doing more research in this area.

**Regression?**

No.

**Risk**

Low. The change only affects skip navigations, whuch are new in 5.0.
@ajcvickers ajcvickers mentioned this issue Oct 4, 2020
Merged
@smitpatel
Copy link
Member

Skip navigations are not valid for cosmos. BindMember method is changing in 6.0
TryExpand methods are for expanding navigations to weak/owned entities. SkipNavigations are not valid for them either.

@smitpatel
Copy link
Member

Query Filter validation is to warn when we do inner join which relies on the fact there are no orphans in presence of query filters for a required relationship is present. That case is not applicable to skip navigations.

ajcvickers added a commit that referenced this issue Oct 5, 2020
@ajcvickers
[5.0] Fix places where skip navigations are missed
4d81e6a 
Fixes #22882

**Description**

Skip navigations (i.e. many-to-many navigations; new for 5.0) are not being returned as members, navigations, or collections from an an API used to access them. In addition, scrubbing the code found two other issues with the same pattern, plus a few that need more investigation. This PR contains fixes and tests for the three clear issues. I have filed #22883 to follow up on the others.

**Customer Impact**

* Navigations for a major new feature are not being returned from an API that should return them.
* Attaching a graph asynchronously may miss navigations
* Skip navigations are missed as notification properties

**How found**

Customer reported on RC1.

**Test coverage**

This PR adds tests for the affected cases, including others found from scrubbing the code. #22883 tracks doing more research in this area.

**Regression?**

No.

**Risk**

Low. The change only affects skip navigations, whuch are new in 5.0.
ajcvickers added a commit that referenced this issue Oct 5, 2020
@ajcvickers
[5.0] Fix places where skip navigations are missed
59a5ef6 
Fixes #22882

**Description**

Skip navigations (i.e. many-to-many navigations; new for 5.0) are not being returned as members, navigations, or collections from an an API used to access them. In addition, scrubbing the code found three other issues with the same pattern, plus a few that need more investigation. This PR contains fixes and tests for the four clear issues. I have filed #22883 to follow up on the others.

**Customer Impact**

* Navigations for a major new feature are not being returned from an API that should return them.
* Attaching a graph asynchronously may miss navigations.
* Skip navigations are missed as notification properties.
* Exception will say a property is not found, when actually it is a navigation.

**How found**

Customer reported on RC2 daily build.

**Test coverage**

This PR adds tests for the affected cases, including others found from scrubbing the code. #22883 tracks doing more research in this area.

**Regression?**

No.

**Risk**

Low. The change only affects skip navigations, which are new in 5.0.
@AndriySvyryd AndriySvyryd added this to the 5.0.0 milestone Oct 6, 2020
@AndriySvyryd AndriySvyryd self-assigned this Oct 6, 2020
ajcvickers added a commit that referenced this issue Oct 6, 2020
@ajcvickers
[5.0] Fix places where skip navigations are missed (#22884)
71063cb 
Fixes #22882

**Description**

Skip navigations (i.e. many-to-many navigations; new for 5.0) are not being returned as members, navigations, or collections from an an API used to access them. In addition, scrubbing the code found three other issues with the same pattern, plus a few that need more investigation. This PR contains fixes and tests for the four clear issues. I have filed #22883 to follow up on the others.

**Customer Impact**

* Navigations for a major new feature are not being returned from an API that should return them.
* Attaching a graph asynchronously may miss navigations.
* Skip navigations are missed as notification properties.
* Exception will say a property is not found, when actually it is a navigation.

**How found**

Customer reported on RC2 daily build.

**Test coverage**

This PR adds tests for the affected cases, including others found from scrubbing the code. #22883 tracks doing more research in this area.

**Regression?**

No.

**Risk**

Low. The change only affects skip navigations, which are new in 5.0.
AndriySvyryd added a commit that referenced this issue Oct 6, 2020
@AndriySvyryd
Add validation for seed data with skip navigations
63756ef 
Part of #22883
@AndriySvyryd AndriySvyryd mentioned this issue Oct 6, 2020
Merged
AndriySvyryd added a commit that referenced this issue Oct 7, 2020
@AndriySvyryd
Add validation for seed data with skip navigations
d3b2ff9 
Part of #22883
@ajcvickers ajcvickers added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. Servicing-approved area-global labels Oct 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-global closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. Servicing-approved type-bug
Projects
None yet
Milestone
5.0.0
Development

No branches or pull requests
3 participants
@ajcvickers @smitpatel @AndriySvyryd