Business central auth fixed #61
Conversation
ruuushhh
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
ruuushhh
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
WalkthroughThe recent updates reflect a restructuring in the management of Business Central credentials within a workspace application. The process for connecting to Business Central has been simplified, with some of the logic being shifted away from serializers to views, indicating a possible move towards a more view-centric approach for handling credentials. 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? TipsChat with CodeRabbit Bot (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (3)
- apps/workspaces/helpers.py (3 hunks)
- apps/workspaces/serializers.py (2 hunks)
- apps/workspaces/views.py (2 hunks)
Additional comments: 5
apps/workspaces/views.py (3)
8-8: The import of
connect_business_centralis correctly placed and follows the Python convention of grouping third-party imports separately from local application imports.81-95: The
postmethod implementation correctly uses theconnect_business_centralfunction to create Business Central credentials and returns a serialized response. Ensure that theauthorization_codeandredirect_uriare validated before use to prevent security issues.97-98: The
get_querysetmethod is implemented correctly, returning all instances ofBusinessCentralCredentials. This is a standard Django ORM practice for views that require a queryset.apps/workspaces/helpers.py (1)
- 88-93: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [88-102]
The
connect_business_centralfunction has been refactored to remove the dependency onWorkspaceattributes. Ensure that the removal of this logic does not affect other parts of the application that may rely on these attributes. The function correctly handles the creation or update ofBusinessCentralCredentials.apps/workspaces/serializers.py (1)
- 84-86: The
createmethod has been removed from theBusinessCentralCredentialsSerializerclass. This change implies that the creation logic has been moved elsewhere, likely to theConnectBusinessCentralViewclass. Verify that this change aligns with the application's design and that the credential creation process is still secure and functional.
|
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #61 +/- ##
==========================================
+ Coverage 90.29% 90.91% +0.61%
==========================================
Files 51 51
Lines 2504 2487 -17
==========================================
Hits 2261 2261
+ Misses 243 226 -17 ☔ View full report in Codecov by Sentry. |
* Company Selection API * test case fixed * moved to business central
ruuushhh
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
|
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (7)
- apps/business_central/helpers.py (2 hunks)
- apps/business_central/serializers.py (1 hunks)
- apps/business_central/urls.py (2 hunks)
- apps/business_central/utils.py (2 hunks)
- apps/business_central/views.py (2 hunks)
- tests/test_business_central/test_views.py (1 hunks)
- tests/test_mappings/test_imports/test_modules/test_expense_custom_fields.py (1 hunks)
Files skipped from review due to trivial changes (1)
- tests/test_mappings/test_imports/test_modules/test_expense_custom_fields.py
Additional comments: 10
apps/business_central/urls.py (2)
3-3: The import statement has been updated to include
CompanySelectionView. Ensure that this view is indeed used within this file and that the import is necessary.12-12: The new URL path for
CompanySelectionViewis correctly added with the namecompany-selection. Ensure that this naming convention is consistent with the rest of the application.apps/business_central/views.py (2)
3-7: The import statements have been updated to include
BusinessCentralFieldSerializer,CompanySelectionSerializer, andImportBusinessCentralAttributesSerializer. Confirm that these serializers are used within this file.28-32: The
CompanySelectionViewclass has been added with aserializer_classattribute set toCompanySelectionSerializer. Ensure that theCompanySelectionSerializeris defined and that it contains the necessary logic for this view to function correctly.tests/test_business_central/test_views.py (1)
- 45-64: The new test function
test_post_company_selectionis added to test the POST request for company selection. Verify that the test covers all necessary scenarios and that the assertions are appropriate for the expected outcomes. Additionally, ensure that the test function name and docstring accurately describe its purpose.apps/business_central/helpers.py (2)
7-7: The import order in
helpers.pyhas been modified. Confirm that this new order follows the project's conventions and that there are no circular dependencies introduced.51-51: The
'companies'dimension has been added to thedimensionslist in thesync_dimensionsfunction. Ensure that this addition is consistent with the intended behavior of the function and that the necessary logic to handle this new dimension is in place.apps/business_central/serializers.py (1)
- 98-121: The
CompanySelectionSerializerclass has been added with aMetainner class and acreatemethod. Ensure that theMetaclass correctly specifies the model and fields, and that thecreatemethod contains the appropriate logic for updatingWorkspaceattributes based on the request data and context.apps/business_central/utils.py (2)
64-71: The logic in the
_sync_datamethod has been modified to include 'COMPANY' in theattribute_typeparameter check and to handle thedisplay_nameparameter using the 'displayName' field if available. Confirm that this logic is correct and that it aligns with the data structure of the items being synchronized.84-86: The
sync_companiesmethod now passes an empty list as thefield_namesparameter to the_sync_datamethod. Ensure that this is the intended behavior and that it does not negatively impact the synchronization process.
Summary by CodeRabbit