Skip to content

New update to history and margins in news card#15

Merged
5RoD merged 14 commits intomasterfrom
dev
May 23, 2025
Merged

New update to history and margins in news card#15
5RoD merged 14 commits intomasterfrom
dev

Conversation

@5RoD
Copy link
Owner

@5RoD 5RoD commented May 7, 2025

This pull request introduces several updates related to the CSS for improved styling, and the inclusion of a new section in the history.php file.

Project Configuration Updates:

  • .idea/BLD.iml: Added spec as a test source folder and excluded multiple vendor directories to streamline project indexing.
  • .idea/php.xml: Updated PHP include paths to include additional vendor libraries and configured PHPUnit with a custom loader path.
  • .idea/phpspec.xml: Introduced a new configuration file for PHPSpec with project-level suite settings.

Dependency Management:

  • composer.json: Added phpunit/phpunit as a development dependency with version constraint ^11.5.

UI and Styling Enhancements:

  • public/css/style.css:
    • Fixed a missing semicolon in the body height property.
    • Added a new .div-margin-news-temp class for margin adjustments.
    • Updated .news-card-text with padding and margin tweaks for better layout.

Content Addition:

  • public/html/history.php: Added a detailed "Sign Up Section" describing the organization's history, partnership with the Esports World Cup Foundation, and future plans.

Summary by CodeRabbit

  • New Features
    • Added a detailed history section about the organization, including recent partnerships and organizational background, to the site.
    • Added a new "History" navigation link to the site header.
    • Introduced an "Agenda & Events" section listing upcoming events with interactive event alerts.
    • Added a new results page displaying match results from the database.
    • Added an admin interface for submitting and storing match results.
  • Style
    • Introduced new CSS classes for improved spacing, layout of news card text, and tooltip UI components.
  • Chores
    • Updated project configuration files for enhanced IDE support and PHP include paths.
    • Added PHPUnit as a development dependency.
    • Updated ignore rules to exclude environment files and IDE/vendor directories from version control.
  • Bug Fixes
    • Improved database connection error messaging for clearer context.
    • Corrected navigation link to the team section.
    • Fixed phone number input field type for proper HTML validation.

@coderabbitai
Copy link

coderabbitai bot commented May 7, 2025

"""

Walkthrough

This update introduces several configuration and content changes. IDE-specific files were updated to expand test and vendor paths, and new PHPSpec configuration was added. Composer's development dependencies now include PHPUnit. The .gitignore now explicitly ignores .env. CSS was modified to add spacing utilities and adjust text styling. The history page was replaced with a static HTML narrative. A new "History" navigation link was added to the header. An agenda section with upcoming events and placeholder tooltips was added. The database connection script was slightly refactored for clarity. A new results PHP script querying match results was introduced.

Changes

File(s) Change Summary
.gitignore Added .idea, vendor, and explicit .env entries to ignored files.
.idea/BLD.iml Added $MODULE_DIR$/spec as a test source folder; excluded multiple vendor directories; added "jquery" library entry.
.idea/php.xml Added many vendor directories to PHP include paths; introduced PhpCodeSniffer, PhpStan, PhpUnit, and Psalm components with interpreter settings and timeouts.
.idea/phpspec.xml New file: Added PHPSpec suite configuration with a path variable.
composer.json Added phpunit/phpunit as a development dependency under require-dev.
public/css/style.css Added .div-margin-news-temp class; updated .news-card-text with new margin and padding; added .titleHistory class; removed redundant semicolon.
public/html/history.php Replaced PHP placeholder with a detailed static HTML news article about BLD's history and partnership.
public/includes/header.php Updated "Team" link to #team; added "History" and "Results" navigation links.
public/html/agenda.php Added "Agenda & Events" section with a table listing three upcoming events and placeholder tooltips for descriptions.
public/js/agenda.js Added empty or whitespace-only content; no functional code added.
public/php/connect.php Refactored comments for clarity; consolidated PDO connection instantiation into one line; enhanced error message with filename.
public/admin/adminContactDashboard.php Simplified require path for database connection file.
public/html/results.php New PHP script querying MatchResults table and outputting results in an HTML table with proper escaping.
public/admin/adminResults.php New PHP script handling form submission to insert match results into the database with input sanitization and prepared statements.
public/php/mysqlTables.php Modified ContactUS table schema: changed contactus_id column from INT(6) to plain INT.
public/html/contactus.php Changed phone input field type from invalid "phonenumber" to valid "number".
public/html/home.php Added id="team" attribute to the team members section for anchor linking.

Possibly related PRs

Poem

In the warren of code, some changes appear—
A .env now hidden, our secrets kept clear.
CSS grows margins, the news cards now gleam,
PHPUnit joins in for a test-driven dream.
History retold in static delight,
Events and alerts bring new insight.
The rabbit hops on—everything’s right!
🐇✨
"""

Note

⚡️ AI Code Reviews for VS Code, Cursor, Windsurf

CodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback.
Learn more here.

✨ Finishing Touches
  • 📝 Generate Docstrings

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@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

🧹 Nitpick comments (6)
public/css/style.css (3)

40-40: Suggest adding semicolon to CSS declaration
While browsers tolerate the missing semicolon on height: 100%, it’s best practice to terminate each property for clarity and to avoid future parsing edge cases.


174-176: Reconsider fixed div-margin-news-temp margins
Hard-coded margins of 50px 500px may break on smaller viewports. Consider using relative or responsive units (e.g., vh, vw, or media queries) to maintain layout consistency.


591-592: Reevaluate large bottom margin on .news-card-text
A margin-bottom: 260px could create excessive whitespace and complicate responsive layouts. Consider a more fluid spacing approach (e.g., margin-bottom: 2rem or CSS variables).

.idea/php.xml (1)

15-42: Consolidate PHP include paths for maintainability.
Listing each vendor subdirectory manually is error-prone and requires updates whenever dependencies change. IntelliJ supports including the entire vendor directory, which covers all current and future packages.

Apply this diff to simplify the include_path:

-  <include_path>
-    <path value="$PROJECT_DIR$/vendor/composer" />
-    <path value="$PROJECT_DIR$/vendor/sebastian/type" />
-    <path value="$PROJECT_DIR$/vendor/sebastian/diff" />
-    <path value="$PROJECT_DIR$/vendor/sebastian/lines-of-code" />
-    <path value="$PROJECT_DIR$/vendor/phar-io/version" />
-    <path value="$PROJECT_DIR$/vendor/sebastian/object-enumerator" />
-    <path value="$PROJECT_DIR$/vendor/phar-io/manifest" />
-    <path value="$PROJECT_DIR$/vendor/sebastian/comparator" />
-    <path value="$PROJECT_DIR$/vendor/sebastian/code-unit" />
-    <path value="$PROJECT_DIR$/vendor/sebastian/exporter" />
-    <path value="$PROJECT_DIR$/vendor/sebastian/cli-parser" />
-    <path value="$PROJECT_DIR$/vendor/sebastian/version" />
-    <path value="$PROJECT_DIR$/vendor/sebastian/complexity" />
-    <path value="$PROJECT_DIR$/vendor/sebastian/recursion-context" />
-    <path value="$PROJECT_DIR$/vendor/sebastian/environment" />
-    <path value="$PROJECT_DIR$/vendor/sebastian/global-state" />
-    <path value="$PROJECT_DIR$/vendor/sebastian/code-unit-reverse-lookup" />
-    <path value="$PROJECT_DIR$/vendor/sebastian/object-reflector" />
-    <path value="$PROJECT_DIR$/vendor/staabm/side-effects-detector" />
-    <path value="$PROJECT_DIR$/vendor/myclabs/deep-copy" />
-    <path value="$PROJECT_DIR$/vendor/theseer/tokenizer" />
-    <path value="$PROJECT_DIR$/vendor/phpunit/php-code-coverage" />
-    <path value="$PROJECT_DIR$/vendor/phpunit/php-file-iterator" />
-    <path value="$PROJECT_DIR$/vendor/phpunit/php-invoker" />
-    <path value="$PROJECT_DIR$/vendor/phpunit/php-text-template" />
-    <path value="$PROJECT_DIR$/vendor/phpunit/phpunit" />
-    <path value="$PROJECT_DIR$/vendor/nikic/php-parser" />
-    <path value="$PROJECT_DIR$/vendor/phpunit/php-timer" />
-  </include_path>
+  <include_path>
+    <path value="$PROJECT_DIR$/vendor" />
+  </include_path>
public/html/history.php (2)

14-14: Use clickable links for contact details.
Plain-text email addresses and Discord handles aren’t user-friendly. Replace them with anchor tags for mailto: and, if available, a Discord invite URL to improve usability.

-                BLD will be seeking players or teams looking to become part of the organization in time for the Esports World Cup 2025. Interested parties can reach out to VP of Operations Mohamad F: 2158177@talnet.nl or via Discord (@5RoD).
+                BLD will be seeking players or teams looking to become part of the organization in time for the Esports World Cup 2025. Interested parties can reach out to VP of Operations Mohamad F at 
+                  <a href="mailto:2158177@talnet.nl">2158177@talnet.nl</a> or via 
+                  <a href="https://discord.gg/your-invite">Discord (@5RoD)</a>.

11-21: Separate distinct content into semantic paragraphs or sections.
Merging the CEO quote, “About BLD” description, and organizational history into a single <p> reduces readability and semantic clarity. Consider splitting them into multiple <p> tags or a dedicated <section> with its own heading.

-                <p>The CEO of BLD, Mohamad F, said:
-                “Today, as many of you have seen, BLD was announced ... allows that reality."
-                <strong>About BLD</strong>
-                BLD is a premiere Los Angeles-based esports organization ... develop a strong, connected community around esports.
-                From 2022 to 2024, BLD became one of the fastest-growing esports organizations on Twitter, YouTube, and Instagram in North America.</p>
+                <p>The CEO of BLD, Mohamad F, said:
+                  “Today, as many of you have seen, BLD was announced ... allows that reality."</p>
+
+                <section class="about-bld">
+                  <h3>About BLD</h3>
+                  <p>BLD is a premiere Los Angeles-based esports organization that fields competitive teams in Counter-Strike and other titles. The organization has a wide range of content creators, including former Counter-Strike major winner Tarik Celik, and notable streamers like Daphne “39Daph” Wai and Jared “Zombs” Gitlin.</p>
+                  <p>BLD Academy is the Counter-Strike team for BLD. We are committed ... pipeline for future BLD stars.</p>
+                  <p>From 2022 to 2024, BLD became one of the fastest-growing esports organizations on Twitter, YouTube, and Instagram in North America.</p>
+                </section>
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2169d02 and 60ae830.

⛔ Files ignored due to path filters (1)
  • composer.lock is excluded by !**/*.lock
📒 Files selected for processing (7)
  • .gitignore (1 hunks)
  • .idea/BLD.iml (1 hunks)
  • .idea/php.xml (1 hunks)
  • .idea/phpspec.xml (1 hunks)
  • composer.json (1 hunks)
  • public/css/style.css (3 hunks)
  • public/html/history.php (1 hunks)
🔇 Additional comments (8)
.gitignore (2)

139-139: Approve ignoring top-level vendor directory
Adding /vendor/ ensures third-party libraries aren’t accidentally committed, which aligns with PHP best practices.


141-141: Approve ignoring .env file
Explicitly ignoring the root .env file complements existing .env.* patterns and safeguards sensitive environment variables.

composer.json (1)

18-20: Approve addition of PHPUnit as a dev dependency
Including "phpunit/phpunit": "^11.5" under require-dev aligns with the IDE test-folder updates and ensures a consistent testing framework across the project.

.idea/phpspec.xml (1)

1-10: Approve new PHPSpec IDE configuration
The .idea/phpspec.xml file correctly sets up a PhpSpec suite pointing at the project root. This complements the PHPUnit and Spec source-folder enhancements.

.idea/BLD.iml (2)

5-5: Approve marking spec as a test source folder
Adding <sourceFolder url="file://$MODULE_DIR$/spec" isTestSource="true" /> will integrate BDD specs into the IDE’s test runner.


7-33: Approve excluding vendor subdirectories from indexing
The long list of <excludeFolder> entries prevents heavy vendor code from polluting project indexing and search. This matches the Composer and PHPUnit setup.

.idea/php.xml (1)

49-53: Verify PhpUnit integration settings.
Ensure that the <PhpUnitSettings custom_loader_path="$PROJECT_DIR$/vendor/autoload.php"/> entry is recognized by your IDE and that vendor/autoload.php actually exists. Also confirm that your project root contains a phpunit.xml or phpunit.xml.dist so that IntelliJ can pick up your test configuration.

public/html/history.php (1)

1-26: Verify inclusion context and document structure.
This file currently contains only a <section> fragment without any <doctype>, <html>, <head>, or <body> tags, and lacks meta tags (charset, viewport). Confirm whether it’s intended as a template fragment that gets injected into a broader layout. If it’s meant to stand alone, wrap it in the proper HTML5 boilerplate and include necessary <meta> tags for responsive design.

Repository owner deleted a comment from coderabbitai bot May 12, 2025
Copy link

@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

🧹 Nitpick comments (1)
public/js/agenda.js (1)

5-5: Replace placeholder alert message

The "Hello World!" alert is a development placeholder and should be replaced with a meaningful message for users.

Consider replacing it with something more descriptive about the action being taken, or remove the alert entirely and implement a more user-friendly interaction.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a0b7aca and 9a34338.

📒 Files selected for processing (5)
  • .gitignore (2 hunks)
  • .idea/php.xml (1 hunks)
  • public/html/agenda.php (1 hunks)
  • public/js/agenda.js (1 hunks)
  • public/php/connect.php (2 hunks)
✅ Files skipped from review due to trivial changes (2)
  • public/php/connect.php
  • public/html/agenda.php
🚧 Files skipped from review as they are similar to previous changes (2)
  • .gitignore
  • .idea/php.xml

Comment on lines 3 to 5
let agenda1 = document.getElementById("agenda1");

agenda1.addEventListener("click", "tbody" ,function(){ alert("Hello World!")}); No newline at end of file
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix event listener implementation and add error handling

There are several issues with the current JavaScript implementation:

  1. The addEventListener method takes only 3 parameters: event type, callback function, and options. The current usage with "tbody" as the second parameter is incorrect.
  2. There's no error handling to check if the element with ID "agenda1" exists before adding the event listener.

Here's a corrected implementation:

-let agenda1 = document.getElementById("agenda1");
-
-agenda1.addEventListener("click", "tbody" ,function(){ alert("Hello World!")});
+document.addEventListener("DOMContentLoaded", function() {
+  const agenda1 = document.getElementById("agenda1");
+  
+  if (agenda1) {
+    // If you want to listen for clicks on the entire agenda1 element
+    agenda1.addEventListener("click", function(){ 
+      alert("Hello World!");
+    });
+    
+    // Or if you want to delegate to tbody children specifically:
+    // agenda1.querySelectorAll("tbody").forEach(tbody => {
+    //   tbody.addEventListener("click", function() {
+    //     alert("Hello World!");
+    //   });
+    // });
+  } else {
+    console.error("Element with ID 'agenda1' not found");
+  }
+});
📝 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
let agenda1 = document.getElementById("agenda1");
agenda1.addEventListener("click", "tbody" ,function(){ alert("Hello World!")});
document.addEventListener("DOMContentLoaded", function() {
const agenda1 = document.getElementById("agenda1");
if (agenda1) {
// If you want to listen for clicks on the entire agenda1 element
agenda1.addEventListener("click", function() {
alert("Hello World!");
});
// Or if you want to delegate to tbody children specifically:
// agenda1.querySelectorAll("tbody").forEach(tbody => {
// tbody.addEventListener("click", function() {
// alert("Hello World!");
// });
// });
} else {
console.error("Element with ID 'agenda1' not found");
}
});

Copy link

@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: 3

🧹 Nitpick comments (2)
public/html/agenda.php (2)

13-27: Enhance table rows for better accessibility and user experience.

The table rows are interactive but lack visual cues and proper accessibility attributes:

  1. Add visual indicators that rows are clickable (cursor style, hover effects)
  2. Improve accessibility with proper ARIA roles and keyboard navigation
  3. Consider using more semantic HTML for interactive elements
-        <tr data-event="speedrun">
+        <tr data-event="speedrun" role="button" tabindex="0" class="clickable-row">

Add to your CSS file:

.clickable-row {
    cursor: pointer;
}
.clickable-row:hover {
    background-color: #f5f5f5;
}

Also update your JavaScript to handle keyboard events:

row.addEventListener("keydown", (e) => {
    if (e.key === "Enter" || e.key === " ") {
        // Trigger the same action as click
        e.preventDefault();
        // Your event handling code here
    }
});

3-3: Consider adding descriptive text for the events section.

The events section has a title but no descriptive text explaining what these events are or how users can participate.

     <h1>Agenda & Events</h1>
+    <p>Join BLD at our upcoming community events! Click on any event for more details or to register.</p>
     <table>
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9a34338 and 827cf9b.

📒 Files selected for processing (3)
  • .idea/BLD.iml (1 hunks)
  • public/html/agenda.php (1 hunks)
  • public/js/agenda.js (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • public/js/agenda.js
  • .idea/BLD.iml

Comment on lines 32 to 49
<script>
// Grab all rows inside #agenda
const rows = document.querySelectorAll("#agenda tr");

rows.forEach((row) => {
row.addEventListener("click", () => {
let event = row.dataset.event;

if (event === "speedrun") {
alert("You clicked speedrun event");
} else if (event === "bbq") {
alert("You clicked bbq event");
} else if (event === "showdown") {
alert("You clicked showdown event");
}
});
});
</script>
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Move JavaScript code to an external file.

The JavaScript code should be moved to the external public/js/agenda.js file that was created in this PR. This follows best practices for code organization, maintainability, and performance (allowing browser caching).

-<script>
-    // Grab all rows inside #agenda
-    const rows = document.querySelectorAll("#agenda tr");
-
-    rows.forEach((row) => {
-        row.addEventListener("click", () => {
-            let event = row.dataset.event;
-
-            if (event === "speedrun") {
-                alert("You clicked speedrun event");
-            } else if (event === "bbq") {
-                alert("You clicked bbq event");
-            } else if (event === "showdown") {
-                alert("You clicked showdown event");
-            }
-        });
-    });
-</script>
+<script src="/js/agenda.js"></script>

Then add this code to public/js/agenda.js:

document.addEventListener('DOMContentLoaded', () => {
    // Grab all rows inside #agenda
    const rows = document.querySelectorAll("#agenda tr");

    rows.forEach((row) => {
        row.addEventListener("click", () => {
            let event = row.dataset.event;

            if (event === "speedrun") {
                alert("You clicked speedrun event");
            } else if (event === "bbq") {
                alert("You clicked bbq event");
            } else if (event === "showdown") {
                alert("You clicked showdown event");
            }
        });
    });
});

Comment on lines 40 to 46
if (event === "speedrun") {
alert("You clicked speedrun event");
} else if (event === "bbq") {
alert("You clicked bbq event");
} else if (event === "showdown") {
alert("You clicked showdown event");
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve user interaction beyond simple alerts.

Using alert() dialogs for user interaction is not a modern or user-friendly approach. Consider:

  1. Showing event details in a modal dialog or expandable section
  2. Linking to dedicated event pages with registration options
  3. Using inline content expansion instead of disruptive alerts

Here's a conceptual example of how this could be improved:

if (event === "speedrun") {
    window.location.href = "/events/speedrun-details.php";
    // OR show details in a modal:
    // showEventModal("speedrun", "BLD Charity Speedrun Showdown", "July 13, 2025", 
    //                "Join us for our charity event supporting...");
} else if (event === "bbq") {
    // Similar implementation
}

Copy link

@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: 3

🧹 Nitpick comments (1)
public/html/results.php (1)

1-26: Restructure file for better separation of concerns

The overall file structure mixes database logic, business logic, and presentation. Consider restructuring to follow better architectural patterns.

Consider implementing the following structure:

  1. Move database queries to a separate service/model layer
  2. Create a controller that handles the business logic
  3. Use templates for the presentation layer

For example:

<?php
// Include necessary files
require_once("../php/connect.php");
require_once("../models/MatchResultsModel.php");

// Controller logic
try {
    $matchResultsModel = new MatchResultsModel($conn);
    $matchResults = $matchResultsModel->getRecentResults(20);
    
    // Include view template with the data
    include("../templates/match-results.php");
} catch (Exception $e) {
    // Log error
    error_log("Error in results.php: " . $e->getMessage());
    
    // Set error message for template
    $errorMessage = "Unable to retrieve match results. Please try again later.";
    include("../templates/error.php");
}
?>
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 827cf9b and 145066e.

📒 Files selected for processing (3)
  • public/admin/adminContactDashboard.php (1 hunks)
  • public/html/results.php (1 hunks)
  • public/php/mysqlTables.php (1 hunks)
✅ Files skipped from review due to trivial changes (2)
  • public/admin/adminContactDashboard.php
  • public/php/mysqlTables.php

mohamad.fallaha added 3 commits May 16, 2025 14:08
@5RoD 5RoD merged commit 9cc078b into master May 23, 2025
1 of 2 checks passed
Copy link

@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: 3

🧹 Nitpick comments (3)
public/html/contactus.php (1)

63-63: Consider using type="tel" for better phone number handling.

While type="number" fixes the invalid "phonenumber" type, type="tel" would be more appropriate for phone numbers as it:

  • Provides better semantic meaning
  • Avoids unwanted spinner controls
  • Handles phone number formatting better on mobile devices
  • Allows special characters like +, -, (), and spaces
-                <input type="number" id="phonenumber" name="phonenumber" placeholder="06 12345678" required maxlength="10">
+                <input type="tel" id="phonenumber" name="phonenumber" placeholder="06 12345678" required maxlength="10">
public/admin/adminResults.php (2)

58-61: Improve score input validation.

The score field should have better input constraints to ensure consistent formatting.

            <div>
                <label for="score">Match Score</label>
-                <input type="text" id="score" name="score" placeholder="?-?" required>
+                <input type="text" id="score" name="score" placeholder="3-1" pattern="^\d+-\d+$" title="Score format: X-Y (e.g., 3-1)" required>
            </div>

2-2: Improve code comment professionalism.

The informal comment should be more professional and descriptive.

-// needed to load the database stuff variables
+// Include database connection configuration
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2bd1314 and 2fd3acf.

📒 Files selected for processing (6)
  • public/admin/adminResults.php (1 hunks)
  • public/html/contactus.php (1 hunks)
  • public/html/home.php (1 hunks)
  • public/html/results.php (1 hunks)
  • public/includes/header.php (1 hunks)
  • public/php/connect.php (2 hunks)
✅ Files skipped from review due to trivial changes (1)
  • public/html/home.php
🚧 Files skipped from review as they are similar to previous changes (3)
  • public/includes/header.php
  • public/php/connect.php
  • public/html/results.php

Comment on lines +36 to +42
<!-- Contact Us Section -->
<section class="contactus">
<div class="contactus-container">
<h2>Admin Results</h2>

<!-- Contact Form -->
<form action="index.php?page=admin/adminResults" method="POST" class="contactus-form">
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Fix copy-paste artifacts from contact form.

The HTML section contains remnants from the contact form template that should be updated for the admin results context.

-<!-- Contact Us Section -->
-<section class="contactus">
-    <div class="contactus-container">
+<!-- Admin Results Section -->
+<section class="admin-results">
+    <div class="admin-container">
         <h2>Admin Results</h2>

-        <!-- Contact Form -->
-        <form action="index.php?page=admin/adminResults" method="POST" class="contactus-form">
+        <!-- Results Form -->
+        <form action="index.php?page=admin/adminResults" method="POST" class="admin-form">
📝 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
<!-- Contact Us Section -->
<section class="contactus">
<div class="contactus-container">
<h2>Admin Results</h2>
<!-- Contact Form -->
<form action="index.php?page=admin/adminResults" method="POST" class="contactus-form">
<!-- Admin Results Section -->
<section class="admin-results">
<div class="admin-container">
<h2>Admin Results</h2>
<!-- Results Form -->
<form action="index.php?page=admin/adminResults" method="POST" class="admin-form">
🤖 Prompt for AI Agents
In public/admin/adminResults.php around lines 36 to 42, the HTML section still
contains leftover elements and comments from a contact form template. Update the
section header, class names, and form attributes to reflect the admin results
context instead of contact form references. Remove or replace any contact
form-specific comments and labels to match the purpose of the admin results
page.

Comment on lines +15 to +19
$game_title = htmlspecialchars($_POST['game_title']);
$winner = htmlspecialchars($_POST['winner']);
$score = htmlspecialchars($_POST['score']);
$match_type = htmlspecialchars($_POST['match_type']);
$match_date = htmlspecialchars($_POST['match_date']);
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add server-side validation for form inputs.

The current implementation only uses htmlspecialchars() for sanitization but lacks proper validation for data types and formats.

Add validation after sanitization:

    // Get and sanitize form data
    $game_title = htmlspecialchars($_POST['game_title']);
    $winner = htmlspecialchars($_POST['winner']);
    $score = htmlspecialchars($_POST['score']);
    $match_type = htmlspecialchars($_POST['match_type']);
    $match_date = htmlspecialchars($_POST['match_date']);

+    // Validate inputs
+    if (empty($game_title) || strlen($game_title) > 100) {
+        die("Invalid game title");
+    }
+    if (empty($winner) || strlen($winner) > 50) {
+        die("Invalid winner name");
+    }
+    if (!preg_match('/^\d+-\d+$/', $score)) {
+        die("Score must be in format 'X-Y'");
+    }
+    if (!DateTime::createFromFormat('Y-m-d', $match_date)) {
+        die("Invalid date format");
+    }
🤖 Prompt for AI Agents
In public/admin/adminResults.php around lines 15 to 19, the form inputs are only
sanitized with htmlspecialchars() but lack server-side validation for data types
and formats. After sanitizing each input, add validation checks such as ensuring
game_title and winner are non-empty strings, score is a valid number or follows
the expected format, match_type matches allowed values, and match_date is a
valid date. If any validation fails, handle the error appropriately by returning
an error message or stopping further processing.

Comment on lines +1 to +33
<?php
// needed to load the database stuff variables
require_once("../php/connect.php");

// Check if form was submitted
if ($_SERVER['REQUEST_METHOD'] === 'POST') {
global $conn;

if (!$conn) {
var_dump($_ENV); // Check if .env is loaded
die("Database connection is not established.");
}

// Get and sanitize form data
$game_title = htmlspecialchars($_POST['game_title']);
$winner = htmlspecialchars($_POST['winner']);
$score = htmlspecialchars($_POST['score']);
$match_type = htmlspecialchars($_POST['match_type']);
$match_date = htmlspecialchars($_POST['match_date']);

try {
// Prepare SQL statement and execute
$stmt = $conn->prepare("INSERT INTO MatchResults (game_title, winner, score, match_type, match_date) VALUES (?, ?, ?, ?, ?)");
$stmt->execute([$game_title, $winner, $score, $match_type, $match_date]);

// Redirect to avoid form re-submission on page reload
header("Location: " . $_SERVER['REQUEST_URI']);
exit();

} catch (PDOException $e) {
echo "Error: " . $e->getMessage();
}
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Critical: Add authentication and authorization checks for admin functionality.

This admin interface lacks any authentication or authorization mechanisms, allowing anyone to access and manipulate match results. This poses a significant security risk.

Add authentication checks before processing the form:

<?php
+// Check if user is authenticated as admin
+session_start();
+if (!isset($_SESSION['user_role']) || $_SESSION['user_role'] !== 'admin') {
+    header('Location: /login.php');
+    exit();
+}

// needed to load the database stuff variables
require_once("../php/connect.php");
🤖 Prompt for AI Agents
In public/admin/adminResults.php lines 1 to 33, there is no authentication or
authorization check before allowing access to the admin functionality, which is
a security risk. Add a session-based authentication check at the start of the
script to verify if the user is logged in and has admin privileges. If the user
is not authenticated or authorized, redirect them to a login page or show an
access denied message, preventing unauthorized access to the form processing and
database operations.

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