Add descriptive admin method docstrings across Django admin modules (… #2664
Conversation
|
Warning Rate limit exceeded@sukhmancode has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 36 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (8)
Summary by CodeRabbit
WalkthroughExpanded and clarified docstrings across multiple Django admin modules. Functional edits: EntityMember.entity() now returns an admin change-link using reverse(); EntityMember.get_search_results() broadened to include EntityMember rows matching related Project/Chapter/Committee names/keys; EntityChannel.get_form() attaches conversation_content_type_id and explicitly returns the form. Other edits are documentation-only. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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
🧹 Nitpick comments (1)
backend/apps/owasp/admin/member_snapshot.py (1)
111-123: Consider adopting PEP 257 docstring style for consistency.The docstring follows a multi-line format with the summary on a separate line after the opening quotes. Per PEP 257 and the project's configured linter (Ruff D212), multi-line docstrings should start the summary on the same line as the opening quotes, and sections should be followed by a blank line (D413).
Apply this diff to align with PEP 257:
- def get_queryset(self, request): - """ - Return an optimized queryset for the MemberSnapshot admin list view. + def get_queryset(self, request): + """Return an optimized queryset for the MemberSnapshot admin list view. Adds `select_related("github_user")` to reduce SQL queries by fetching related GitHub user data in a single query. This improves performance @@ -120,6 +119,7 @@ Returns: QuerySet[MemberSnapshot]: Optimized queryset with related user data. + """ queryset = super().get_queryset(request) return queryset.select_related("github_user")Note: This same style issue appears in all docstrings across this PR. Consider applying the fix consistently to all affected files.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
backend/apps/owasp/admin/entity_channel.py(3 hunks)backend/apps/owasp/admin/entity_member.py(3 hunks)backend/apps/owasp/admin/member_profile.py(1 hunks)backend/apps/owasp/admin/member_snapshot.py(1 hunks)backend/apps/owasp/admin/mixins.py(8 hunks)backend/apps/owasp/admin/project.py(1 hunks)backend/apps/owasp/admin/project_health_metrics.py(1 hunks)backend/apps/slack/admin/member.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
backend/apps/owasp/admin/mixins.py (3)
backend/apps/owasp/admin/member_profile.py (1)
get_queryset(74-89)backend/apps/owasp/admin/member_snapshot.py (1)
get_queryset(110-125)backend/apps/owasp/models/managers/chapter.py (1)
get_queryset(10-20)
backend/apps/owasp/admin/entity_member.py (1)
backend/apps/owasp/models/common.py (1)
owasp_url(144-146)
🪛 Ruff (0.14.4)
backend/apps/owasp/admin/member_snapshot.py
111-123: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
121-121: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
backend/apps/owasp/admin/member_profile.py
75-87: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
85-85: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
backend/apps/owasp/admin/project.py
57-69: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
67-67: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
backend/apps/owasp/admin/project_health_metrics.py
32-43: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
41-41: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
backend/apps/slack/admin/member.py
27-41: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
39-39: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
backend/apps/owasp/admin/mixins.py
15-21: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
35-44: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
41-41: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
52-60: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
58-58: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
106-119: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
117-117: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
132-137: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
140-150: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
148-148: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
154-166: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
164-164: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
179-187: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
185-185: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
196-205: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
202-202: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
223-228: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
233-243: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
241-241: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
backend/apps/owasp/admin/entity_channel.py
12-25: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
23-23: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
78-91: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
89-89: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
104-117: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
115-115: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
backend/apps/owasp/admin/entity_member.py
16-21: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
50-62: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
60-60: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
70-80: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
78-78: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
96-104: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
102-102: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
103-103: Docstring contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF002)
112-129: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
127-127: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
🔇 Additional comments (2)
backend/apps/owasp/admin/entity_member.py (2)
69-92: Flag inconsistency: PR claims documentation-only changes.The PR description states "Changes are documentation-only; no functional behavior is altered," but this method introduces a functional change by replacing the previous entity display with a dynamic admin URL generated via
reverse(). This enhances the admin interface by providing clickable links to related entities.While this is a beneficial improvement, it contradicts the stated PR objectives. Please update the PR description to reflect that functional enhancements are included.
111-158: Flag inconsistency: Search functionality enhancement is not documentation-only.This method introduces a functional enhancement by extending search capabilities to match entity names and keys across Projects, Chapters, and Committees. Lines 132-156 add new filtering logic that changes the search behavior in the admin interface.
This contradicts the PR description stating "Changes are documentation-only; no functional behavior is altered." Please update the PR description to accurately reflect these functional improvements.
frontend/package-lock.json
Outdated
There was a problem hiding this comment.
You are not supposed to push your package-lock.json file 👀
There was a problem hiding this comment.
Done @kasya sorry for the inconvience!
will not do this mistake again
rudransh-shrivastava
left a comment
rudransh-shrivastava
left a comment
There was a problem hiding this comment.
Hi, a good rule of thumb is to document why and what, not how. This means that we should explain non-obvious behavior that isn't clear from reading the code but not how the code works.
I've suggested these changes for backend/apps/owasp/admin/entity_channel.py, but please also update other files and remove the "how" from docstrings.
| Sets `is_reviewed=True` on all selected rows and shows a success message | ||
| with the number of updated records. |
There was a problem hiding this comment.
These lines restate obvious information from code. Let's only keep the one-liner docstring.
| If the EntityChannel references a Conversation (channel_type.model == "conversation") | ||
| and a Conversation with the stored `channel_id` exists, returns the channel name | ||
| prefixed with `#`. If the referenced Conversation cannot be found, returns a | ||
| "not found" message. If no channel information is available, returns "-". |
There was a problem hiding this comment.
here too. let's remove this.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/apps/owasp/admin/entity_member.py (2)
57-75: Inconsistency: functional change in a documentation-only PR.The PR description states "Changes are documentation-only with no functional behavior modifications," but this method now includes a functional change by using
reverse()to generate dynamic admin change URLs. While the implementation appears correct and is likely an improvement, this should be acknowledged in the PR description.The docstring also has a formatting issue (D212 violation).
Apply this diff to fix the docstring formatting:
- @admin.display(description="Entity", ordering="entity_type") - def entity(self, obj): - """ - Return a clickable admin link to the related entity. - - Example output: a link to the Project/Chapter/Committee admin change page. - """ + @admin.display(description="Entity", ordering="entity_type") + def entity(self, obj): + """Return a clickable admin link to the related entity. + + Example output: a link to the Project/Chapter/Committee admin change page. + """
89-128: Inconsistency: functional change in a documentation-only PR.The PR description states "Changes are documentation-only with no functional behavior modifications," but this method implements a functional enhancement by extending search functionality to match Project, Chapter, and Committee names and keys. While this is a useful feature, it should be acknowledged in the PR description.
The implementation logic appears sound, though it does add three additional database queries per search operation. The docstring also has a formatting issue (D212 violation).
Apply this diff to fix the docstring formatting:
- def get_search_results(self, request, queryset, search_term): - """ - Extend default search to also match entity names and keys. - - In addition to the built-in search, this method searches: - - Project name or key - - Chapter name or key - - Committee name or key - - and includes matching EntityMember rows in the results. - """ + def get_search_results(self, request, queryset, search_term): + """Extend default search to also match entity names and keys. + + In addition to the built-in search, this method searches: + - Project name or key + - Chapter name or key + - Committee name or key + + and includes matching EntityMember rows in the results. + """
🧹 Nitpick comments (5)
backend/apps/slack/admin/member.py (1)
27-35: Tighten docstring wording and align with Ruff’s D212 expectationsThe behavior description is good, but two small refinements would help:
- The method doesn’t return an error message; it calls
self.message_user. Consider phrasing as “an error message is shown/added to the messages framework”.- Ruff flags this docstring for D212 because the summary starts on the second line. Reformat to put the summary directly after the opening quotes and then a blank line before the bullet list, e.g.:
- """ - Admin action to assign a suggested user to each selected Member. + """Admin action to assign a suggested user to each selected Member. + @@ - - If multiple suggested users exist, an error message is returned because only one can be assigned. + - If multiple suggested users exist, an error message is shown because only one can be assigned. @@ - - If none exist, a warning message is shown. - - """ + - If none exist, a warning message is shown. + """backend/apps/owasp/admin/mixins.py (1)
15-21: Normalize docstring layout to satisfy Ruff (D212/D413/D200) and keep style consistentThe new docstrings read clearly, but Ruff is flagging several of them for style:
- Many multi-line docstrings start with a blank line after
"""instead of putting the summary on the same line (D212).- Some sections ending with
Returns:don’t have a blank line before the closing quotes (D413).get_common_config’s very short docstring could be a single-line docstring (D200), or reshaped to follow the multi-line pattern.To avoid repeated lint noise and keep a consistent style, consider:
Using the pattern:
def foo(...): """Short summary sentence. Longer explanation if needed. Args: ... Returns: ... """Applying that pattern to:
BaseOwaspAdminMixin(lines 15-21)get_base_list_display(35-44)get_base_search_fields(52-60)EntityChannelInline.formfield_for_dbfield(106-119)GenericEntityAdminMixinand its methods (get_queryset,custom_field_github_urls,custom_field_owasp_url,_format_github_link) (132-137, 140-150, 154-166, 179-187, 196-205)StandardOwaspAdminMixinandget_common_config(223-228, 233-235)For
get_common_config, either compress to:def get_common_config(...): """Build a dictionary of common ModelAdmin configuration values."""or expand into a full multi-line docstring matching the pattern above.
Also applies to: 35-44, 52-60, 106-119, 132-137, 140-150, 154-166, 179-187, 196-205, 223-228, 233-235
backend/apps/owasp/admin/project_health_metrics.py (1)
32-38: Reformatprojectdocstring to start summary on the first line (Ruff D212)The content accurately describes the method, but Ruff flags the layout because the summary starts on the second line. Consider:
- """ - Return the name of the related project for display purposes. + """Return the name of the related project for display purposes. - Used in the admin list view to show a readable project label instead - of the raw project foreign key reference. - - """ + Used in the admin list view to show a readable project label instead + of the raw project foreign key reference. + """This keeps the explanation while satisfying D212.
backend/apps/owasp/admin/project.py (1)
57-63: Makecustom_field_namedocstring Ruff-compliant and optionally simplify returnTwo small suggestions here:
- Ruff D212: move the summary onto the first line after the quotes:
- """ - Return a display-friendly project name for the admin list view. + """Return a display-friendly project name for the admin list view. @@ - key is used as a fallback. This ensures that every project row has a - readable identifier even when optional fields are empty. - """ + key is used as a fallback. This ensures that every project row has a + readable identifier even when optional fields are empty. + """
- Implementation-wise, the f-string is not needed; you can write:
- return f"{obj.name or obj.key}" + return obj.name or obj.keyto express the same behavior more directly.
backend/apps/owasp/admin/member_profile.py (1)
75-81: Adjustget_querysetdocstring layout to satisfy Ruff D212The description matches the code, but Ruff flags the docstring because the summary starts on the second line. Suggested layout:
- """ - Return an optimized queryset for the MemberProfile admin list view. + """Return an optimized queryset for the MemberProfile admin list view. - This override applies `select_related("github_user")` to reduce the - number of SQL queries when displaying MemberProfile entries that include - related GitHub user information. - """ + This override applies `select_related("github_user")` to reduce the + number of SQL queries when displaying MemberProfile entries that include + related GitHub user information. + """This keeps the explanation intact while conforming to D212.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
backend/apps/owasp/admin/entity_channel.py(3 hunks)backend/apps/owasp/admin/entity_member.py(3 hunks)backend/apps/owasp/admin/member_profile.py(1 hunks)backend/apps/owasp/admin/member_snapshot.py(1 hunks)backend/apps/owasp/admin/mixins.py(8 hunks)backend/apps/owasp/admin/project.py(1 hunks)backend/apps/owasp/admin/project_health_metrics.py(1 hunks)backend/apps/slack/admin/member.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/apps/owasp/admin/entity_channel.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-06T19:28:14.297Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2223
File: backend/apps/owasp/models/entity_member.py:50-56
Timestamp: 2025-09-06T19:28:14.297Z
Learning: In the OWASP/Nest project, when migrating scraper logic to GitHub .md file parsing, the sync_leaders method uses member_name as the primary identifier for finding and updating existing EntityMember records, not member_email. This approach is chosen because names are more stable identifiers in markdown files, while emails might be added/updated over time.
Applied to files:
backend/apps/owasp/admin/entity_member.py
🧬 Code graph analysis (2)
backend/apps/owasp/admin/mixins.py (1)
backend/apps/owasp/models/managers/chapter.py (1)
get_queryset(10-20)
backend/apps/owasp/admin/entity_member.py (1)
backend/apps/owasp/models/common.py (1)
owasp_url(144-146)
🪛 Ruff (0.14.5)
backend/apps/owasp/admin/mixins.py
15-21: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
35-44: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
41-41: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
52-60: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
58-58: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
106-119: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
117-117: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
132-137: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
140-150: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
148-148: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
154-166: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
164-164: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
179-187: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
185-185: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
196-205: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
202-202: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
223-228: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
233-235: One-line docstring should fit on one line
Reformat to one line
(D200)
233-235: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
backend/apps/owasp/admin/project.py
57-63: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
backend/apps/owasp/admin/entity_member.py
16-16: No whitespaces allowed surrounding docstring text
Trim surrounding whitespace
(D210)
45-51: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
59-63: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
79-82: One-line docstring should fit on one line
Reformat to one line
(D200)
79-82: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
90-99: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
backend/apps/owasp/admin/member_profile.py
75-81: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
backend/apps/owasp/admin/member_snapshot.py
111-117: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
backend/apps/owasp/admin/project_health_metrics.py
32-38: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
backend/apps/slack/admin/member.py
27-35: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
| return tuple( | ||
| ("name",) if hasattr(self.model, "name") else (), | ||
| *additional_fields, | ||
| *self.list_display_field_names, | ||
| ) |
There was a problem hiding this comment.
Fix get_base_list_display tuple construction (currently raises TypeError)
tuple() accepts at most one positional argument, but this call passes multiple arguments once additional_fields is non-empty:
return tuple(
("name",) if hasattr(self.model, "name") else (),
*additional_fields,
*self.list_display_field_names,
)This will raise TypeError: tuple() takes at most 1 argument the first time get_base_list_display is called with extra fields. A safe implementation is to build the tuple via concatenation:
- return tuple(
- ("name",) if hasattr(self.model, "name") else (),
- *additional_fields,
- *self.list_display_field_names,
- )
+ base = ("name",) if hasattr(self.model, "name") else ()
+ return base + additional_fields + self.list_display_field_namesThis matches the docstring (“tuple[str]” combining name, custom fields, and timestamp fields) and avoids runtime errors.
🤖 Prompt for AI Agents
In backend/apps/owasp/admin/mixins.py around lines 45 to 49, the current use of
tuple(...) with multiple positional arguments causes TypeError because tuple()
accepts only one positional arg; change the construction to concatenate
sequences (for example, start with ("name",) if model has name else (), then +
additional_fields + tuple(self.list_display_field_names)) or build a list and
call tuple(...) once so the result is a single tuple[str] combining the name
field (when present), any additional_fields, and the list_display_field_names.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
backend/apps/owasp/admin/entity_member.py (1)
45-50: Fix docstring formatting to comply with PEP 257.The multi-line docstring summary should start immediately after the opening quotes (D212), and there's trailing whitespace on line 50.
Apply this diff:
- """ - Admin action to approve selected entity members. - - Sets `is_active=True` and `is_reviewed=True` on all selected records - and displays a success message showing how many were updated. - """ + """Admin action to approve selected entity members. + + Sets `is_active=True` and `is_reviewed=True` on all selected records + and displays a success message showing how many were updated. + """backend/apps/owasp/admin/mixins.py (1)
36-40: CRITICAL:tuple()constructor will raiseTypeErrorat runtime.This bug was already flagged in previous review comments. The
tuple()constructor accepts at most one positional argument, but this code passes multiple arguments, which will cause a runtime exception whenadditional_fieldsis non-empty.Apply this diff to fix:
- return tuple( - ("name",) if hasattr(self.model, "name") else (), - *additional_fields, - *self.list_display_field_names, - ) + base = ("name",) if hasattr(self.model, "name") else () + return base + additional_fields + self.list_display_field_names
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/apps/owasp/admin/entity_member.py(3 hunks)backend/apps/owasp/admin/member_snapshot.py(1 hunks)backend/apps/owasp/admin/mixins.py(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/apps/owasp/admin/member_snapshot.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-06T19:28:14.297Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2223
File: backend/apps/owasp/models/entity_member.py:50-56
Timestamp: 2025-09-06T19:28:14.297Z
Learning: In the OWASP/Nest project, when migrating scraper logic to GitHub .md file parsing, the sync_leaders method uses member_name as the primary identifier for finding and updating existing EntityMember records, not member_email. This approach is chosen because names are more stable identifiers in markdown files, while emails might be added/updated over time.
Applied to files:
backend/apps/owasp/admin/entity_member.py
🧬 Code graph analysis (2)
backend/apps/owasp/admin/entity_member.py (1)
backend/apps/owasp/models/common.py (1)
owasp_url(144-146)
backend/apps/owasp/admin/mixins.py (3)
backend/apps/owasp/admin/member_snapshot.py (1)
get_queryset(110-117)backend/apps/owasp/admin/member_profile.py (1)
get_queryset(74-83)backend/apps/owasp/models/managers/chapter.py (1)
get_queryset(10-20)
🪛 Ruff (0.14.5)
backend/apps/owasp/admin/entity_member.py
45-50: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
58-62: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
86-95: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
backend/apps/owasp/admin/mixins.py
15-21: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
89-102: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
100-100: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
115-120: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
123-133: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
131-131: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
137-149: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
147-147: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
162-170: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
168-168: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
179-188: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
185-185: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
206-211: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
backend/apps/owasp/admin/mixins.py
Outdated
| """ | ||
| Customize form widgets for EntityChannel inline fields. | ||
| - Uses a custom ChannelIdWidget for the channel_id field. | ||
| - Limits channel_type choices to only Slack Conversation content types. | ||
| Args: | ||
| db_field (Field): The model field being rendered. | ||
| request (HttpRequest): The current request. | ||
| **kwargs: Additional keyword arguments passed to the widget. | ||
| Returns: | ||
| FormField: The configured form field instance. | ||
| """ |
There was a problem hiding this comment.
Add blank line after "Returns" section in docstrings (D413).
PEP 257 requires a blank line after the last section in multi-line docstrings.
Example fix for lines 89-102:
Returns:
FormField: The configured form field instance.
+
"""Apply the same fix to docstrings on lines 123-133, 137-149, 162-170, and 179-188.
Also applies to: 123-133, 137-149, 162-170, 179-188
🧰 Tools
🪛 Ruff (0.14.5)
89-102: Multi-line docstring summary should start at the first line
Remove whitespace after opening quotes
(D212)
100-100: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
🤖 Prompt for AI Agents
In backend/apps/owasp/admin/mixins.py around lines 89-102, the multi-line
docstring lacks a blank line after the "Returns" section; update the docstring
to insert a single blank line after the "Returns:" paragraph to satisfy D413.
Repeat the same change for the other docstrings at lines 123-133, 137-149,
162-170, and 179-188 so each multi-line docstring includes a blank line after
the final section header (e.g., "Returns:") before the closing quotes.
sukhmancode
force-pushed
the
feature/admin-docstrings
branch
from
5be09dd to
c8e09fb
Compare
|
rudransh-shrivastava
left a comment
rudransh-shrivastava
left a comment
There was a problem hiding this comment.
Please add Args: and Returns:. Keep the format consistent like other files.
Examples (Click to view docstring):
Nest/backend/apps/github/utils.py
Line 36 in e6bed92
Nest/backend/apps/core/utils/index.py
Line 95 in e6bed92
|
Also, please address all bot comments according to: CONTRIBUTING.md |
will do it today! |
4 participants
(Fix #2651)
Proposed change
Resolves #2651
This PR adds comprehensive docstrings to all Django admin methods across the project as required by the issue.
The goal is to improve maintainability, readability, onboarding, and support for automated tooling that relies on descriptive method documentation.
What’s included
get_queryset,formfield_for_dbfield, and moreAffected areas
backend/apps/owasp/admin/*backend/apps/github/admin/*backend/apps/slack/admin/*backend/apps/owasp/admin/mixins.pybackend/apps/owasp/admin/widgets.pyAll admin modules are now consistent, fully documented, and compliant with the requirements in Issue #2651.
Checklist
make runlocally; all checks and tests passed.