Skip to content

Cleanup: dead APIs, unescaped output, missing index, N+1 listings, unused strings #22

Description

@nihaalshaikh

Summary

Low-priority maintainability items surfaced by the review:

Dead / removed core APIs

  • lib.php:539 pcast_cron() and version.php $plugin->cron (pre-3.0 cron).
  • lib.php:639 pcast_get_completion_state() - superseded by classes/completion/custom_completion.php.
  • lib.php:553 pcast_scale_used() - deprecated per-module callback.
  • showepisode.php:58 $CFG->framename; db/log.php legacy logging; db/uninstall.php defines mod_pcast_uninstall() (core calls xmldb_pcast_uninstall()).

Escaping / SQL

  • lib.php:523, locallib.php:1050 use format_text() on the plain-text name (should be format_string()); locallib.php:1183/:1209 emit name/category unescaped.
  • lib.php:467 builds IN (...) via implode(); rsslib.php:217, 252, 274 interpolate ints into SQL; rsslib.php:470-477, 551, 612 build XML attributes without escaping.
  • db/access.php:73-94 - mod/pcast:write / :manage lack RISK_XSS.

Performance

  • db/install.xml - no index on pcast_episodes.pcastid/userid.
  • locallib.php:503-971 - the five pcast_display_*_episodes() functions load the entire episode set then paginate in PHP, with per-episode user (:1087) and tag (:1099) lookups (N+1).

Other

  • mod_form.php:118 default 2 for a 0/1 select; :275 file_get_submitted_draft_itemid('id') should be 'image'.
  • Explicit-content labels inverted vs the RSS side (mod_form.php:166-168, edit_form.php:126-128).
  • Completion state passed as COMPLETION_COMPLETE after delete/disapprove (deleteepisode.php:124, approveepisode.php:81) - should be COMPLETION_UNKNOWN.
  • ~14 defined-but-unused language strings (approvalview, commentsnotenabled, episodetitle, explainaddentry, pcastactivityname, pcastadministration, pcastfieldset, tagsdeleted, tagtitle, userscanrate, userscanrate_help, viewnewepisodes, viewpcast, viewunapprovedepisodes).

Suggested fix

Address opportunistically alongside the higher-severity work.


Found during the mod_pcast review/test pass (ClickUp 86cb46r9t). Line numbers are against master @ 73bb255.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workinglowLow: minor / cleanup

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions