Skip to content

Fix #2414: Add ExclusionModel support to profile export#2434

Open
Sayeem3051 wants to merge 2 commits intoborgbase:masterfrom
Sayeem3051:patch-1
Open

Fix #2414: Add ExclusionModel support to profile export#2434
Sayeem3051 wants to merge 2 commits intoborgbase:masterfrom
Sayeem3051:patch-1

Conversation

@Sayeem3051
Copy link

Added ExclusionModel handling to profile export and database methods.

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have read the CONTRIBUTING guide.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

I provide my contribution under the terms of the license of this repository and I affirm the Developer Certificate of Origin.

Added ExclusionModel handling to profile export and database methods.
@m3nu
Copy link
Contributor

m3nu commented Mar 15, 2026

This looks better, but you sniped the issue away from someone else who asked to work on it properly. So we'll give him time to come up with their own PR. If they end up not working on it, we can consider this.

#2414 (comment)

Working on an open source project also includes understanding the context and working with other contributors. It's not that the fastest will win.

@m3nu m3nu marked this pull request as draft March 15, 2026 13:01
@Sayeem3051
Copy link
Author

Thank you for the feedback, @m3nu. I understand and completely agree - I should have checked the issue context more carefully before working on it. I see now that someone else had already asked to work on this issue.

I apologize for jumping ahead without consulting with the community first. You're absolutely right that collaboration and understanding the context is important in open source projects. It's not about speed, but about being a good team member and respecting others' contributions.

I'll close this PR and let the other contributor have the opportunity to work on it. If they don't end up completing it, I'd be happy to contribute then.

Thank you for the learning opportunity - I'll make sure to check issue discussions and be more thoughtful about picking up issues in the future.

@m3nu m3nu marked this pull request as ready for review March 19, 2026 07:18
@m3nu
Copy link
Contributor

m3nu commented Mar 19, 2026

Code review

Found 3 issues:

  1. Corrupted from_db method definition. The def from_db(cls, ...) line is replaced with a bare integer 55 and a detached parameter tuple (cls, profile, ...). This is a SyntaxError that prevents the module from loading. Likely a copy-paste artifact that included a line number from a file viewer.

@classmethod
55
(cls, profile, store_password=True, include_settings=True):

  1. Profile FK not set on ExclusionModel rows before insert in to_db. The SourceFileModel pattern explicitly reassigns the profile FK to the (potentially remapped) ID before inserting. Without this, exclusions are inserted with the original exported profile ID, which may differ after collision resolution. They become orphaned or associated with the wrong profile.

SourceFileModel.insert_many(self._profile_dict['SourceFileModel']).execute()
# Insert ExclusionModel
ExclusionModel.insert_many(self._profile_dict['ExclusionModel']).execute()

The established pattern to follow (SourceFileModel, same file):

# Set the profile ids to be match new profile
for source in self._profile_dict['SourceFileModel']:
source['profile'] = self.id
# Delete existing Sources to avoid duplicates

  1. No delete of existing ExclusionModel rows before insert_many in to_db. The SourceFileModel pattern runs delete().where(profile == self.id).execute() before inserting to prevent duplicate rows on re-import. This guard was added via commit 22a21f08 to fix a known duplication bug. The ExclusionModel block omits this step.

SourceFileModel.insert_many(self._profile_dict['SourceFileModel']).execute()
# Insert ExclusionModel
ExclusionModel.insert_many(self._profile_dict['ExclusionModel']).execute()

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants