[Network Drive] fix duplicate ids in access control sync

moxarth-elastic commented 1 week ago

Closes https://github.com/elastic/connectors/issues/2821

Network Drive connector is currently indexing the access control docs (user docs) with group ids and causes duplicate ids for documents within that group. So, this PR now assigning the user id instead of group id.

Checklists

Pre-Review Checklist

[x] this PR does NOT contain credentials of any kind, such as API keys or username/passwords (double check config.yml.example)
[x] this PR has a meaningful title
[x] this PR links to all relevant github issues that it fixes or partially addresses
[x] if there is no GH issue, please create it. Each PR should have a link to an issue
[x] this PR has a thorough description
[x] Covered the changes with automated tests
[x] Tested the changes locally
[x] Added a label for each target release version (example: v7.13.2, v7.14.0, v8.0.0)
[ ] Considered corresponding documentation changes
[ ] Contributed any configuration settings changes to the configuration reference
[ ] if you added or changed Rich Configurable Fields for a Native Connector, you made a corresponding PR in Kibana

Release Note

Fixing the overwriting issue for access control docs.

moxarth-elastic commented 1 week ago

I don't think this is changing anything, is it? I don't see any logical change, just some syntax changes.

Actually there is a logical change i.e. rid at this line https://github.com/elastic/connectors/blob/e1f8f80eac01bcbb0b5ef7cb1e9c2850bd6e127f/connectors/sources/network_drive.py#L721 was initialised and overwritten in the for loop and that was causing this problem. So, we removed it and made this change https://github.com/elastic/connectors/blob/e1f8f80eac01bcbb0b5ef7cb1e9c2850bd6e127f/connectors/sources/network_drive.py#L727

there's no test change. Please make sure that any fix is illustrated with a test.

Sure, we'll add a test case for the changes.

seanstory commented 1 week ago

Ah! Thanks, didn't see that rid was defined already a few lines above the diff. Thanks for clarifying. 👍

github-actions[bot] commented 5 days ago

💔 Failed to create backport PR(s)

Status	Branch	Result
✅	8.x	https://github.com/elastic/connectors/pull/2845
❌	8.15	Commit could not be cherrypicked due to conflicts

Successful backport PRs will be merged automatically after passing CI.

To backport manually run: backport --pr 2832 --autoMerge --autoMergeMethod squash

seanstory commented 4 days ago

@moxarth-elastic can you please manually backport to the 8.15 branch?

moxarth-elastic commented 3 days ago

@moxarth-elastic can you please manually backport to the 8.15 branch?

@seanstory here is the backport PR: https://github.com/elastic/connectors/pull/2848, please take a look.

elastic / connectors