-
Notifications
You must be signed in to change notification settings - Fork 0
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
Handle change of import_settings #357
Conversation
WalkthroughThe recent update enhances the handling of mapping settings in a workspace application. It introduces a new trigger function in Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
def __unset_auto_mapped_flag(self, current_mapping_settings: List[MappingSetting], new_mappings_settings: List[Dict]): | ||
""" | ||
Set the auto_mapped flag to false for the expense_attributes for the attributes | ||
whose mapping is changed. | ||
""" | ||
changed_source_fields = [] | ||
|
||
for new_setting in new_mappings_settings: | ||
destination_field = new_setting['destination_field'] | ||
source_field = new_setting['source_field'] | ||
current_setting = current_mapping_settings.filter(destination_field=destination_field).first() | ||
if current_setting and current_setting.source_field != source_field: | ||
changed_source_fields.append(source_field) | ||
|
||
ExpenseAttribute.objects.filter(workspace_id=self.__workspace_id, attribute_type__in=changed_source_fields).update(auto_mapped=False) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The method __unset_auto_mapped_flag
is well-implemented for adjusting the auto_mapped
flags. However, the documentation incorrectly mentions "Post save action" which should be corrected to reflect its actual usage in a pre-save context.
- Post save action for mapping settings
+ Pre-save action for adjusting auto_mapped flags
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
def __unset_auto_mapped_flag(self, current_mapping_settings: List[MappingSetting], new_mappings_settings: List[Dict]): | |
""" | |
Set the auto_mapped flag to false for the expense_attributes for the attributes | |
whose mapping is changed. | |
""" | |
changed_source_fields = [] | |
for new_setting in new_mappings_settings: | |
destination_field = new_setting['destination_field'] | |
source_field = new_setting['source_field'] | |
current_setting = current_mapping_settings.filter(destination_field=destination_field).first() | |
if current_setting and current_setting.source_field != source_field: | |
changed_source_fields.append(source_field) | |
ExpenseAttribute.objects.filter(workspace_id=self.__workspace_id, attribute_type__in=changed_source_fields).update(auto_mapped=False) | |
def __unset_auto_mapped_flag(self, current_mapping_settings: List[MappingSetting], new_mappings_settings: List[Dict]): | |
""" | |
Pre-save action for adjusting auto_mapped flags | |
""" | |
changed_source_fields = [] | |
for new_setting in new_mappings_settings: | |
destination_field = new_setting['destination_field'] | |
source_field = new_setting['source_field'] | |
current_setting = current_mapping_settings.filter(destination_field=destination_field).first() | |
if current_setting and current_setting.source_field != source_field: | |
changed_source_fields.append(source_field) | |
ExpenseAttribute.objects.filter(workspace_id=self.__workspace_id, attribute_type__in=changed_source_fields).update(auto_mapped=False) |
def pre_save_mapping_settings(self): | ||
""" | ||
Post save action for mapping settings | ||
""" | ||
mapping_settings = self.__mapping_settings | ||
|
||
current_mapping_settings = MappingSetting.objects.filter(workspace_id=self.__workspace_id).all() | ||
self.__unset_auto_mapped_flag(current_mapping_settings, mapping_settings) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The method pre_save_mapping_settings
is correctly implemented to handle pre-save actions for mapping settings. However, the documentation should be updated to accurately describe its role as a pre-save action.
- Post save action for mapping settings
+ Pre-save action for mapping settings
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
def pre_save_mapping_settings(self): | |
""" | |
Post save action for mapping settings | |
""" | |
mapping_settings = self.__mapping_settings | |
current_mapping_settings = MappingSetting.objects.filter(workspace_id=self.__workspace_id).all() | |
self.__unset_auto_mapped_flag(current_mapping_settings, mapping_settings) | |
def pre_save_mapping_settings(self): | |
""" | |
Pre-save action for mapping settings | |
""" | |
mapping_settings = self.__mapping_settings | |
current_mapping_settings = MappingSetting.objects.filter(workspace_id=self.__workspace_id).all() | |
self.__unset_auto_mapped_flag(current_mapping_settings, mapping_settings) |
|
No description provided.