On duplicate form re-generate slug #703
Conversation
WalkthroughThe changes update the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FormController
participant FormModel
User->>FormController: Request to duplicate form (id)
FormController->>FormModel: Retrieve original form
alt Original slug is a UUID
FormController->>FormModel: Generate new UUID slug
else Original slug is not a UUID
FormController->>FormModel: Set slug to null
FormController->>FormModel: Save form to obtain an ID
FormController->>FormModel: Generate new slug
end
FormController->>FormModel: Reset removed_properties to empty array
FormController->>User: Return duplicated form
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
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 (
|
![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)
api/app/Http/Controllers/Forms/FormController.php (1)
188-195: Consider wrapping slug generation in a transaction.The slug generation logic is well-implemented, handling both UUID and non-UUID cases correctly. However, consider wrapping the operations in a database transaction to ensure atomicity.
public function duplicate($id) { $form = Form::findOrFail($id); $this->authorize('update', $form); - // Create copy - $formCopy = $form->replicate(); - // generate new slug before changing title - if (Str::isUuid($formCopy->slug)) { - $formCopy->slug = Str::uuid(); - } else { - $formCopy->slug = null; // Reset the slug first - $formCopy->save(); // Save to ensure we have an ID - $formCopy->generateSlug(); // Now generate the slug - } - $formCopy->title = 'Copy of ' . $formCopy->title; - $formCopy->removed_properties = []; - $formCopy->save(); + return DB::transaction(function () use ($form) { + // Create copy + $formCopy = $form->replicate(); + // generate new slug before changing title + if (Str::isUuid($formCopy->slug)) { + $formCopy->slug = Str::uuid(); + } else { + $formCopy->slug = null; // Reset the slug first + $formCopy->save(); // Save to ensure we have an ID + $formCopy->generateSlug(); // Now generate the slug + } + $formCopy->title = 'Copy of ' . $formCopy->title; + $formCopy->removed_properties = []; + $formCopy->save(); + + return $this->success([ + 'message' => 'Form successfully duplicated. You are now editing the duplicated version of the form.', + 'new_form' => new FormResource($formCopy), + ]); + }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
api/app/Http/Controllers/Forms/FormController.php(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build the Nuxt app
🔇 Additional comments (1)
api/app/Http/Controllers/Forms/FormController.php (1)
197-197: LGTM! Good practice to reset removed_properties.Explicitly resetting
removed_propertiesto an empty array ensures a clean state for the duplicated form.
Summary by CodeRabbit