-
Notifications
You must be signed in to change notification settings - Fork 47
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
fix: Editing of existing snippet not allowed (unversioned snippets) #177
Conversation
Reviewer's Guide by SourceryThe pull request fixes a bug that prevented editing existing snippets if they were not versioned. The fix removes versioning-specific logic from the snippet form's clean method, ensuring that the uniqueness validation works correctly for all snippets. Sequence diagram for snippet editing validationsequenceDiagram
participant User as User
participant Form as SnippetForm
participant DB as Database
User->>Form: Submit edited snippet
Form->>DB: Query existing snippets
Note right of Form: Now excludes only by snippet_grouper,
Note right of Form: removed versioning check
DB-->>Form: Return matching snippets
Form->>Form: Validate name uniqueness
alt Name is unique
Form-->>User: Accept changes
else Name exists
Form-->>User: Show validation error
end
Class diagram showing SnippetForm changesclassDiagram
class SnippetForm {
+clean()
}
note for SnippetForm "Simplified validation logic:
Removed versioning checks
Only excludes by snippet_grouper"
class Snippet {
+name: str
+slug: str
+snippet_grouper: SnippetGrouper
}
class SnippetGrouper {
}
SnippetForm ..> Snippet: validates
Snippet --> SnippetGrouper: belongs to
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @fsbraun - I've reviewed your changes - here's some feedback:
Overall Comments:
- Please add documentation explaining why it's safe to remove the version checking logic and always exclude the snippet_grouper. This will help with future maintenance.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Description
Fix for #176
Related resources
Checklist
master
Discord to find a “pr review buddy” who is
going to review my pull request.
Summary by Sourcery
Bug Fixes: