fix: resolve issues with initdev command #2051
Conversation
WalkthroughThe changes in this pull request involve modifications to a YAML template and enhancements to the user creation process in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Command
participant ImportJob
User->>Command: run command with --reset-password
Command->>Command: _handle()
Command->>Command: _create_user()
alt User exists
Command->>Command: reset_password=True
else New user
Command->>Command: create user
end
Command->>ImportJob: _create_import_job()
alt Case exists
ImportJob->>Command: ACTION_UPDATE
else No case
ImportJob->>Command: ACTION_CREATE
end
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
deps-report 🔍Commit scanned: 1891fb5 Vulnerable dependencies4 dependencies have vulnerabilities 😱
Outdated dependencies55 outdated dependencies found (including 20 outdated major versions)😢
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
backend/varfish/users/management/commands/data/Case_1.grch37.yaml.tpl (1)
Template variables need to be synchronized in Case_1.grch37.yaml.tpl
The verification confirms that there is indeed an inconsistency in the template variables:
{{ case_name }}is used forfamilyIdin the pedigree section{{ data_case }}is still used in multiple places:
- File paths:
file://{{ data_path }}/{{ data_case }}.grch37.gatk_hc.vcf.gz- Individual identifiers:
{{ data_case }}_index,{{ data_case }}_father,{{ data_case }}_motherThese inconsistencies should be resolved by using the same template variable throughout the file to ensure proper template rendering.
🔗 Analysis chain
Line range hint
143-160: Template variable inconsistency detected.While the pedigree section has been updated to use
{{ case_name }}, other sections still use{{ data_case }}. This inconsistency could lead to errors.Let's verify all occurrences of both variables:
Let me fix the regex pattern for the search.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for remaining instances of data_case and case_name echo "Occurrences of data_case:" rg "{{ data_case }}" -A 1 echo -e "\nOccurrences of case_name:" rg "{{ case_name }}" -A 1Length of output: 365
Script:
#!/bin/bash # Search for remaining instances of data_case and case_name with escaped curly braces echo "Occurrences of data_case:" rg "\{\{ data_case \}\}" -A 1 echo -e "\nOccurrences of case_name:" rg "\{\{ case_name \}\}" -A 1Length of output: 2743
backend/varfish/users/management/commands/initdev.py (2)
412-413: Consider enhancing error handling for case existence check.The case existence check could benefit from additional error handling. Consider adding validation to ensure the case being updated matches the expected schema version.
# Check whether the case already exists. -case_exists = Case.objects.filter(project=project, name=case_name).exists() +try: + case = Case.objects.get(project=project, name=case_name) + case_exists = True + if case.case_version != 2: + raise CommandError(f"Existing case {case_name} has incompatible version {case.case_version}") +except Case.DoesNotExist: + case_exists = False
438-442: Enhance success message to reflect the action type.The success message could be more informative by indicating whether the job was created for updating or creating a case.
- self.stderr.write(self.style.SUCCESS("Created import job.")) + action_type = "update" if case_exists else "create" + self.stderr.write(self.style.SUCCESS(f"Created import job to {action_type} case {case_name}."))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- backend/varfish/users/management/commands/data/Case_1.grch37.yaml.tpl (1 hunks)
- backend/varfish/users/management/commands/initdev.py (8 hunks)
🔇 Additional comments (4)
backend/varfish/users/management/commands/data/Case_1.grch37.yaml.tpl (1)
Line range hint
1-180: Inconsistent genome build versions detected.The file contains mixed references to genome builds:
- Filename indicates
grch37- File attributes specify
grch38in multiple locations- Final VCF section specifies
grch37This inconsistency could cause issues with data processing and analysis.
Let's verify all genome build references:
backend/varfish/users/management/commands/initdev.py (3)
91-91: LGTM: Well-documented command line argument addition.The
--reset-passwordflag is appropriately implemented as a boolean option with a clear help message.
124-124: LGTM: Consistent propagation of reset_password parameter.The reset_password parameter is correctly propagated to all user creation calls with appropriate type annotation.
Also applies to: 134-140
221-229: Consider adding password complexity requirements.While using UUID for password generation is secure, consider implementing password complexity requirements for production environments. This could be important if these development accounts are ever used in staging environments.
Let's check if there are any password-related settings or middleware in the project:
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2051 +/- ##
=====================================
Coverage 91% 91%
=====================================
Files 675 675
Lines 38433 38433
=====================================
Hits 35086 35086
Misses 3347 3347 |
Summary by CodeRabbit
New Features
Bug Fixes
These updates improve user management and maintain data integrity within the application.