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

Populate location in Exchange container resolver #2828

Merged
merged 7 commits into from
Mar 29, 2023

Conversation

ashmrtn
Copy link
Contributor

@ashmrtn ashmrtn commented Mar 16, 2023

Always generate both a path of IDs and a path of
display names for folders in the container resolver.

Changes in internal/connector/exchange/service_functions.go
keep the behavior of the overall system from changing


Does this PR need a docs update or release note?

  • ✅ Yes, it's included
  • 🕐 Yes, but in a later PR
  • ⛔ No

Type of change

  • 🌻 Feature
  • 🐛 Bugfix
  • 🗺️ Documentation
  • 🤖 Test
  • 💻 CI/Deployment
  • 🧹 Tech Debt/Cleanup

Issue(s)

merge after:

Test Plan

  • 💪 Manual
  • ⚡ Unit test
  • 💚 E2E

@aviator-app
Copy link
Contributor

aviator-app bot commented Mar 16, 2023

Current Aviator status

Aviator will automatically update this comment as the status of the PR changes.
Comment /aviator refresh to force Aviator to re-examine your PR (or learn about other /aviator commands).

This PR was merged using Aviator.

@ashmrtn ashmrtn changed the base branch from main to 2486-mail-resolver-cleanup March 16, 2023 00:22
Copy link
Contributor

@ryanfkeepers ryanfkeepers left a comment

Choose a reason for hiding this comment

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

This is great, thank you for backfilling the functionality!

src/internal/connector/exchange/container_resolver.go Outdated Show resolved Hide resolved
Base automatically changed from 2486-mail-resolver-cleanup to main March 23, 2023 15:27
@ashmrtn ashmrtn force-pushed the 2486-populate-resolver-locs branch from 535bbc9 to 6a787b8 Compare March 27, 2023 22:39
@ashmrtn ashmrtn temporarily deployed to Testing March 27, 2023 22:39 — with GitHub Actions Inactive
@ashmrtn ashmrtn temporarily deployed to Testing March 27, 2023 22:39 — with GitHub Actions Inactive
@ashmrtn ashmrtn temporarily deployed to Testing March 27, 2023 22:39 — with GitHub Actions Inactive
@ashmrtn ashmrtn temporarily deployed to Testing March 27, 2023 22:39 — with GitHub Actions Inactive
@ashmrtn ashmrtn temporarily deployed to Testing March 27, 2023 22:39 — with GitHub Actions Inactive
@aviator-app aviator-app bot temporarily deployed to Testing March 28, 2023 00:31 Inactive
@aviator-app aviator-app bot temporarily deployed to Testing March 28, 2023 00:31 Inactive
@aviator-app aviator-app bot temporarily deployed to Testing March 28, 2023 00:31 Inactive
@aviator-app aviator-app bot temporarily deployed to Testing March 28, 2023 00:40 Inactive
@aviator-app aviator-app bot temporarily deployed to Testing March 28, 2023 00:40 Inactive
@aviator-app aviator-app bot temporarily deployed to Testing March 28, 2023 01:21 Inactive
@aviator-app aviator-app bot temporarily deployed to Testing March 28, 2023 01:21 Inactive
@aviator-app aviator-app bot temporarily deployed to Testing March 28, 2023 01:21 Inactive
@aviator-app aviator-app bot temporarily deployed to Testing March 28, 2023 01:22 Inactive
@aviator-app aviator-app bot temporarily deployed to Testing March 28, 2023 01:22 Inactive
@aviator-app aviator-app bot added the blocked Upstream item prevents completion label Mar 28, 2023
@aviator-app
Copy link
Contributor

aviator-app bot commented Mar 28, 2023

This pull request failed to merge: some CI status(es) failed. Remove the blocked label to re-queue.

Failed CI(s): Test-Suite-Trusted

Container resolver indexes by ID already so just populate the location
path for all items and have the other path use IDs only. This means any
consumer of the container resolver can request either:
  * a path consisting of folder IDs only
  * a path consisting of folder display names only

Update some of the surrounding code to perform path comparisons properly
when looking at Exchange folders and return a location path where a
location path is expected. This preserves previous behavior for mail and
contacts.
Since all caches now have both ID and display name paths, remove the
bool that would help switch between the two.
Allow looking up a display name path in the cache since PathInCache uses
the ID path format. Adjust tests and code accordingly.
@ashmrtn ashmrtn force-pushed the 2486-populate-resolver-locs branch from f981cfb to 946df98 Compare March 28, 2023 22:55
@ashmrtn ashmrtn temporarily deployed to Testing March 28, 2023 22:55 — with GitHub Actions Inactive
@ashmrtn ashmrtn temporarily deployed to Testing March 28, 2023 22:55 — with GitHub Actions Inactive
@ashmrtn ashmrtn temporarily deployed to Testing March 28, 2023 22:55 — with GitHub Actions Inactive
@ashmrtn ashmrtn temporarily deployed to Testing March 28, 2023 22:56 — with GitHub Actions Inactive
@ashmrtn ashmrtn temporarily deployed to Testing March 28, 2023 22:56 — with GitHub Actions Inactive
@ashmrtn ashmrtn temporarily deployed to Testing March 28, 2023 22:56 — with GitHub Actions Inactive
@ashmrtn ashmrtn removed the blocked Upstream item prevents completion label Mar 28, 2023
@aviator-app aviator-app bot temporarily deployed to Testing March 29, 2023 00:33 Inactive
@aviator-app aviator-app bot temporarily deployed to Testing March 29, 2023 00:33 Inactive
@sonarcloud
Copy link

sonarcloud bot commented Mar 29, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
8.6% 8.6% Duplication

@aviator-app aviator-app bot temporarily deployed to Testing March 29, 2023 00:34 Inactive
@aviator-app aviator-app bot temporarily deployed to Testing March 29, 2023 00:35 Inactive
@aviator-app aviator-app bot temporarily deployed to Testing March 29, 2023 00:35 Inactive
@aviator-app aviator-app bot temporarily deployed to Testing March 29, 2023 00:35 Inactive
@aviator-app aviator-app bot temporarily deployed to Testing March 29, 2023 00:35 Inactive
@aviator-app aviator-app bot merged commit 91e0a27 into main Mar 29, 2023
@aviator-app aviator-app bot deleted the 2486-populate-resolver-locs branch March 29, 2023 01:20
aviator-app bot pushed a commit that referenced this pull request Apr 3, 2023
Always populate the location field of backup details
for Exchange data types

Fix bug where ParentPath for calendar items used
folder IDs instead of display names

---

#### Does this PR need a docs update or release note?

- [x] ✅ Yes, it's included
- [ ] 🕐 Yes, but in a later PR
- [ ] ⛔ No

#### Type of change

- [x] 🌻 Feature
- [x] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [ ] 🤖 Test
- [ ] 💻 CI/Deployment
- [ ] 🧹 Tech Debt/Cleanup

#### Issue(s)

* #2486
* closes #2827

merge after:
* #2826
* #2808
* #2828

#### Test Plan

- [x] 💪 Manual
- [ ] ⚡ Unit test
- [ ] 💚 E2E
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants