Skip to content

PR-2: Add CPT-aware page templates (MVP parity)#24

Open
NishilHoogar wants to merge 2 commits intofossasia:mainfrom
NishilHoogar:feature/pr-2-templates)
Open

PR-2: Add CPT-aware page templates (MVP parity)#24
NishilHoogar wants to merge 2 commits intofossasia:mainfrom
NishilHoogar:feature/pr-2-templates)

Conversation

@NishilHoogar
Copy link
Collaborator

  • Added 6 page templates under templates/ to render CPT content dynamically.
  • Updated loader to point to these templates.
  • Removed all JSON-based data references.
  • This is the MVP parity implementation for PR-2.

- Added 6 page templates under templates/ to render CPT content dynamically.
- Updated loader to point to these templates.
- Removed all JSON-based data references.
- This is the MVP parity implementation for PR-2.
Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Sorry @NishilHoogar, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

Copy link
Member

@mariobehling mariobehling left a comment

Choose a reason for hiding this comment

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

Thanks for the effort here, but this PR diverges in multiple key areas from the agreed plugin skeleton and the incremental roadmap (PR 1 → PR 5).
Right now, it re-introduces deleted project files, breaks the folder naming conventions, and mixes several concerns in one submission.

Let’s keep the good direction — using CPT-aware page templates — but bring it back onto the standard structure.

🚫 Blocking Issues (must fix before merge)

  1. Repo hygiene files deleted
    This PR removes .github/workflows/phpcs.yml, .gitignore, LICENSE, and other meta files.
    → Please restore them; they are mandatory for CI, linting, and license compliance.

  2. Wrong plugin name / folder structure
    Files such as wpfaevent.php, fossasia-landing.php, and class-wpfaevent-* break consistency.
    → The canonical structure is

    wpfa-event/
      ├─ wpfa-event.php
      ├─ includes/
      ├─ admin/
      ├─ public/
      └─ languages/
    

    All classes must be prefixed WPFA_, and the text domain is wpfa-event.

  3. Templates placed incorrectly
    The page templates are committed under includes/ and other folders.
    → Move them into public/templates/ and register them through a loader class (see WPFA_Templates pattern from PR 4 issue).

  4. Uninstall file in wrong location
    public/uninstall.php must be removed.
    → Only keep a single uninstall.php in the plugin root.

  5. Duplicated boilerplate classes
    Classes like class-wpfaevent-loader.php and class-fossasia-landing-loader.php re-implement functionality already covered by the existing skeleton.
    → Reuse WPFA, WPFA_Loader, WPFA_i18n, and WPFA_Public from PR 1.

  6. CPT usage not aligned with PR 2
    Templates should query existing CPTs wpfa_event and wpfa_speaker and use meta keys:

    • wpfa_event_start_date, wpfa_event_end_date, wpfa_event_location, wpfa_event_url, wpfa_event_speakers
    • wpfa_speaker_org, wpfa_speaker_position, wpfa_speaker_photo, wpfa_speaker_events
      → Remove all JSON or hardcoded arrays; use WP_Query + get_post_meta().
  7. Bundled media
    The PR commits binary files such as images/logo.png.
    → Delete these. Use remote placeholder URLs or the upcoming seeder/Settings API.

  8. Mixed scopes
    The PR adds new public templates and touches admin code and boilerplate.
    → Split into smaller PRs; this one should focus only on PR 4 scope (public rendering).

🧭 How to fix (summary)

  1. Restore removed repo meta files and CI.

  2. Rebase on main; use the wpfa-event/ skeleton.

  3. Move the six page templates to public/templates/:

    • page-landing.php
    • page-speakers.php
    • page-events.php
    • page-past-events.php
    • page-schedule.php
    • page-code-of-conduct.php
  4. Add / fix the loader class WPFA_Templates in includes/class-wpfa-templates.php.

  5. Ensure all output is escaped and translated (esc_html(), esc_url(), __() with text domain wpfa-event).

  6. Remove bundled media, duplicate boilerplate, and public/uninstall.php.

  7. Verify each template queries CPT/meta correctly and uses placeholders for images.

  8. Run PHPCS; ensure all files are lint-clean.

💡 Suggested commit flow

git checkout -b fix/pr24-cleanup

restore meta files, move templates, add loader, clean images

git push -u origin fix/pr24-cleanup


Then open a new PR titled

> feat(public): add CPT-aware page templates (Landing/Speakers/Events/Past/Schedule/CoC)


### ✅ Acceptance checklist for resubmission

* [ ] `.github/`, `LICENSE`, `.gitignore`, and PHPCS configs restored
* [ ] Uses `wpfa-event/` folder and `wpfa-event.php` entry point
* [ ] All classes prefixed `WPFA_`; text domain = `wpfa-event`
* [ ] Six templates in `public/templates/…` registered via `WPFA_Templates`
* [ ] Templates query CPT data; no JSON arrays or bundled media
* [ ] Escaping and i18n applied everywhere
* [ ] `uninstall.php` only at root
* [ ] PHPCS passes; CI green


### 🧱 Example snippet for loader (if missing)

```php
require_once WPFA_EVENT_PLUGIN_DIR . 'includes/class-wpfa-templates.php';
WPFA_Templates::init();

🗒 Summary

This PR is on the right conceptual track (CPT-aware templates) but technically off the rails. Please realign with the standard skeleton and the staged roadmap. Once you re-organize the files as outlined, the review can proceed smoothly. Thanks for the continued work — this cleanup will make future reviews much faster and easier to merge!

Copy link
Member

@mariobehling mariobehling left a comment

Choose a reason for hiding this comment

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

Here’s the exact, copy-pasteable fix plan (with git moves/renames + tiny code stubs) to turn PR #24 into a clean PR that matches PR1–PR4.

0) Create a safety branch

git fetch origin
git checkout -b fix/pr24-cleanup

1) Restore repo hygiene files removed by the PR

(keep CI, license, ignores, phpcs config)

git checkout origin/main -- \
  .github/workflows/phpcs.yml \
  .gitignore \
  LICENSE \
  phpcs.xml \
  README.md \
  readme.txt
git add .
git commit -m "chore: restore repo metadata and CI removed by PR24"

If a path isn’t present on origin/main, skip it—keep going.

2) Ensure the canonical plugin root and names

We standardize on wpfa-event/ with wpfa-event.php and WPFA_* classes.

2.1 Create the proper folders if missing

mkdir -p wpfa-event/{includes,admin/css,admin/js,public/{css,js,templates},languages}

2.2 Rename the root plugin file (if PR used wpfaevent.php or similar)

# If present:
git mv wpfaevent.php wpfa-event/wpfa-event.php 2>/dev/null || true
git mv fossasia-landing.php wpfa-event/wpfa-event.php 2>/dev/null || true

2.3 Move shared classes into includes/ and align prefixes

# Examples—adjust names if your PR used slightly different filenames
git mv includes/class-wpfaevent-loader.php wpfa-event/includes/class-wpfa-loader.php 2>/dev/null || true
git mv includes/class-wpfaevent-i18n.php   wpfa-event/includes/class-wpfa-i18n.php 2>/dev/null || true
git mv includes/class-fossasia-landing-loader.php wpfa-event/includes/class-wpfa-templates.php 2>/dev/null || true

If you don’t have these exact files, skip the git mv; we’ll add the minimal loader below.

3) Put page templates where they belong

PR4 requires six page templates in public/templates/.

# Move any templates that currently live under includes/ or elsewhere:
for f in includes/page-landing.php includes/page-speakers.php \
         includes/page-events.php includes/page-past-events.php \
         includes/page-schedule.php includes/page-code-of-conduct.php; do
  [ -f "$f" ] && git mv "$f" "wpfa-event/public/templates/$(basename $f)"
done

# If the PR used other names, map them:
# e.g. includes/landing-page.php -> public/templates/page-landing.php
[ -f includes/landing-page.php ] && git mv includes/landing-page.php wpfa-event/public/templates/page-landing.php
[ -f includes/speakers-page.php ] && git mv includes/speakers-page.php wpfa-event/public/templates/page-speakers.php
[ -f includes/events-listing-page.php ] && git mv includes/events-listing-page.php wpfa-event/public/templates/page-events.php
[ -f includes/past-events-page.php ] && git mv includes/past-events-page.php wpfa-event/public/templates/page-past-events.php
[ -f includes/schedule-page.php ] && git mv includes/schedule-page.php wpfa-event/public/templates/page-schedule.php
[ -f includes/code-of-conduct-page.php ] && git mv includes/code-of-conduct-page.php wpfa-event/public/templates/page-code-of-conduct.php

Delete accidental machine-path files (Windows path embedded in filename):

git rm -f --cached --ignore-unmatch wpfa-event/public/templates/c_Users_* 2>/dev/null || true
git rm -f --cached --ignore-unmatch includes/c_Users_* 2>/dev/null || true

4) Keep uninstall only at plugin root; remove duplicates

# If uninstall.php ended up under /public/, move it back or remove duplicate
if [ -f public/uninstall.php ]; then
  git rm public/uninstall.php
fi

# Ensure root uninstall file exists (add a minimal one if missing)
if [ ! -f wpfa-event/uninstall.php ]; then
  cat > wpfa-event/uninstall.php <<'PHP'
<?php
if ( ! defined( 'WP_UNINSTALL_PLUGIN' ) ) { exit; }
// delete_option('wpfa_settings'); // uncomment when PR5 settings land
PHP
  git add wpfa-event/uninstall.php
fi
git commit -m "chore: keep uninstall at plugin root only"

5) Minimal template loader (add if missing)

Create wpfa-event/includes/class-wpfa-templates.php (or replace misnamed one):

cat > wpfa-event/includes/class-wpfa-templates.php <<'PHP'
<?php
class WPFA_Templates {
  private static $templates = [
    'page-landing.php'         => 'WPFA – Landing',
    'page-speakers.php'        => 'WPFA – Speakers',
    'page-events.php'          => 'WPFA – Events',
    'page-past-events.php'     => 'WPFA – Past Events',
    'page-schedule.php'        => 'WPFA – Schedule',
    'page-code-of-conduct.php' => 'WPFA – Code of Conduct',
  ];
  public static function init() {
    add_filter( 'theme_page_templates', [ __CLASS__, 'register' ] );
    add_filter( 'template_include',     [ __CLASS__, 'load' ] );
  }
  public static function register( $templates ) {
    foreach ( self::$templates as $file => $label ) { $templates[ $file ] = $label; }
    return $templates;
  }
  public static function load( $template ) {
    if ( is_singular( 'page' ) ) {
      $slug = get_page_template_slug( get_queried_object_id() );
      if ( isset( self::$templates[ $slug ] ) ) {
        $candidate = WPFA_EVENT_PLUGIN_DIR . 'public/templates/' . $slug;
        if ( file_exists( $candidate ) ) { return $candidate; }
      }
    }
    return $template;
  }
}
PHP
git add wpfa-event/includes/class-wpfa-templates.php
git commit -m "feat(public): add WPFA_Templates loader for page templates"

Wire it in wpfa-event/wpfa-event.php (ensure these lines exist—add if missing):

require_once WPFA_EVENT_PLUGIN_DIR . 'includes/class-wpfa-templates.php';
WPFA_Templates::init();

Commit if you edited the file.


6) Prefixes, text domain, and file headers (quick checks)

6.1 Ensure text domain is wpfa-event and template headers are valid

Run a quick search:

grep -RIn --exclude-dir=.git -e "Text Domain:" -e "Template Name:" -e "wpfaevent" .
  • Fix Text Domain: to wpfa-event.
  • Replace any wpfaevent class/file prefixes with WPFA_ / wpfa-….

Optional mass-replace (review diffs before committing):

# Class prefixes
git grep -l "class-wpfaevent" | xargs -r sed -i 's/class-wpfaevent/class-wpfa/g'
git grep -l "WPFAEvent" | xargs -r sed -i 's/WPFAEvent/WPFA/g'

# Text domain
git grep -l "wpfaevent" | xargs -r sed -i 's/wpfaevent/wpfa-event/g'

Stage and commit:

git add -A
git commit -m "refactor: align names to WPFA_* and text domain wpfa-event"

7) No media in git

Remove any committed images/logos:

git rm -f --cached --ignore-unmatch images/* assets/images/* wpfa-event/images/* 2>/dev/null || true
git commit -m "chore: remove bundled media (use placeholders or seeder)"

8) Make sure templates read CPT + meta (PR2 parity)

Each template must query:

  • wpfa_speaker with meta: wpfa_speaker_org, wpfa_speaker_position, wpfa_speaker_photo
  • wpfa_event with meta: wpfa_event_start_date, wpfa_event_end_date, wpfa_event_location, wpfa_event_url, relations (wpfa_event_speakers)

If any template still uses JSON or hardcoded arrays, replace with WP_Query and get_post_meta() calls (you can copy from the PR4 issue we drafted).

Commit those edits with:

git add -A
git commit -m "fix(templates): query CPTs and meta; remove JSON/hardcoded data"

9) PHPCS + quick self-check

# If you use the WPCS workflow we restored, the CI will run on push/PR.
# Local run (if phpcs installed):
phpcs

10) Push and open a focused PR (PR4 scope only)

git push -u origin fix/pr24-cleanup

PR title:

feat(public): add CPT-aware page templates (Landing/Speakers/Events/Past/Schedule/CoC) using WPFA loader

PR description (paste):

  • Keeps repo metadata/CI intact.
  • Uses existing wpfa-event/ skeleton; WPFA_* classes; text domain wpfa-event.
  • Adds six page templates under public/templates/ and registers them via WPFA_Templates.
  • Templates query CPTs/meta from PR2; no JSON or bundled images.
  • uninstall.php lives only at plugin root.
  • PHPCS passes.

Checklist:

  • Repo hygiene files restored
  • Naming aligned (wpfa-event.php, WPFA_*, wpfa-event text domain)
  • Six templates in public/templates/… + loader
  • No media committed; placeholders only
  • Uses CPT/meta; no JSON arrays
  • CI green / PHPCS clean

Copy link
Member

@mariobehling mariobehling left a comment

Choose a reason for hiding this comment

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

Please resolve conflicts.

This PR implements the backend data model as requested in PR Task 2.
Scope is strictly limited to registering the Event and Speaker data structure within the existing WPFAevent plugin architecture.

###  Included
- Registers the `wpfaevent_event` Custom Post Type.
- Registers the `wpfaevent_speaker` Custom Post Type.
- Adds custom capability mappings for both CPTs with `map_meta_cap`.
- Adds Event taxonomies: `wpfaevent_track` (hierarchical) and `wpfaevent_tag` (non-hierarchical).
- Adds meta fields for speaker (position, organization, bio, headshot_url).
- Adds meta fields for event (talk_title, schedule_reference).
- All CPT, taxonomy, and meta registration is fully REST-enabled.
- Integrates all hooks using the existing WPFAevent Loader class.
- Adds initial unit tests for CPT, taxonomy, and meta registration.

###  Not included (per project guidelines)
- No UI or template changes
- No modifications to admin/public assets
- No structural changes to plugin architecture
- No renaming or deletion of existing files
- LICENSE.md and all JS/CSS assets are untouched

###  Notes
This PR focuses solely on the backend data model as specified.
All additions follow the naming conventions and file organization of the existing WPFAevent plugin.
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