Skip to content

Added Schema check in Announcements plugin.#132

Closed
drkthunder02 wants to merge 1 commit into
pelican-dev:mainfrom
drkthunder02:announcements-update
Closed

Added Schema check in Announcements plugin.#132
drkthunder02 wants to merge 1 commit into
pelican-dev:mainfrom
drkthunder02:announcements-update

Conversation

@drkthunder02
Copy link
Copy Markdown

@drkthunder02 drkthunder02 commented May 12, 2026

Updated the Schema for the migration for the Announcements plugin to check whether the table is already loaded into the database. This will prevent errors from cropping up when doing the Auto-Update script.

Summary by CodeRabbit

  • Chores
    • Improved database migration stability with enhanced conditional checks to prevent redundant operations during setup.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 12, 2026

📝 Walkthrough

Walkthrough

Database migration for the announcements table now conditionally creates the table only if it does not exist, preventing duplicate table errors on repeated runs. The schema definition and rollback behavior remain unchanged.

Changes

Announcements Migration Safety

Layer / File(s) Summary
Conditional Table Creation
announcements/database/migrations/001_create_announcements_table.php
The up() method wraps Schema::create('announcements', ...) in a Schema::hasTable('announcements') guard condition, making the migration safely re-runnable. Table schema and down() method unchanged.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A migration wise, now checks with care,
Before creating tables in the air,
No duplicate errors shall arise,
This idempotent path is clean and bright! 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Added Schema check in Announcements plugin' accurately describes the main change: adding a conditional Schema::hasTable() check to prevent re-creating an existing announcements table in the migration.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@announcements/database/migrations/001_create_announcements_table.php`:
- Line 11: The conditional spacing for the table-check is violating Pint rules;
update the if statement that calls Schema::hasTable('announcements') to follow
Laravel/Pint spacing (e.g. change "if(!Schema::hasTable('announcements')) {" to
"if (! Schema::hasTable('announcements')) {" or the project-preferred variant
"if (!Schema::hasTable('announcements')) {"), modifying the line that contains
Schema::hasTable('announcements') in the migration to match the linter's
expected spacing.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4dae6552-c121-4139-83dc-8ec411102d6d

📥 Commits

Reviewing files that changed from the base of the PR and between 587828b and 6125565.

📒 Files selected for processing (1)
  • announcements/database/migrations/001_create_announcements_table.php
📜 Review details
🧰 Additional context used
🪛 GitHub Actions: Lint / 3_Pint.txt
announcements/database/migrations/001_create_announcements_table.php

[error] 1-1: Laravel Pint failed: 1 style issue found in the specified file. CI step 'vendor/bin/pint --test' exited with code 1.

🪛 GitHub Actions: Lint / Pint
announcements/database/migrations/001_create_announcements_table.php

[error] 1-1: Laravel Pint (vendor/bin/pint --test) failed: 1 style issue found in this file. CI exited with code 1.

$table->timestamp('valid_to')->nullable();
$table->timestamps();
});
if(!Schema::hasTable('announcements')) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix Pint style violation on the conditional (currently blocking CI).

Line 11 should follow Laravel/Pint spacing conventions; this is likely the single lint failure in the pipeline.

Suggested fix
-        if(!Schema::hasTable('announcements')) {
+        if (! Schema::hasTable('announcements')) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if(!Schema::hasTable('announcements')) {
if (! Schema::hasTable('announcements')) {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@announcements/database/migrations/001_create_announcements_table.php` at line
11, The conditional spacing for the table-check is violating Pint rules; update
the if statement that calls Schema::hasTable('announcements') to follow
Laravel/Pint spacing (e.g. change "if(!Schema::hasTable('announcements')) {" to
"if (! Schema::hasTable('announcements')) {" or the project-preferred variant
"if (!Schema::hasTable('announcements')) {"), modifying the line that contains
Schema::hasTable('announcements') in the migration to match the linter's
expected spacing.

@Boy132
Copy link
Copy Markdown
Member

Boy132 commented May 12, 2026

Thanks but this is merely a workaround to patch only a sympton. If the migrations runs again when updating the panel it seems it didn't properly the first when the plugin got installed. Laravel keeps track of what migrations ran and will only run them once. So this is just a sympton for a failed/interrupted installation or a different kind of bug.

@Boy132 Boy132 closed this May 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants