Skip to content

Fixing Codechecker for Moodle 5.0. - #763

Open
lucaboesch wants to merge 217 commits into
PoetOS:REFACTOR_500_V2from
lucaboesch:REFACTOR_500_V2_codechecker
Open

Fixing Codechecker for Moodle 5.0.#763
lucaboesch wants to merge 217 commits into
PoetOS:REFACTOR_500_V2from
lucaboesch:REFACTOR_500_V2_codechecker

Conversation

@lucaboesch

Copy link
Copy Markdown
Contributor

No description provided.

mchurchward and others added 30 commits May 22, 2026 14:25
Add Moodle persistent classes for the four core tables in
classes/local/db/: questionnaire_record, survey_record,
question_record, choice_record.

Rename all DB columns that used underscores to comply with Moodle
coding standards (e.g. type_id -> typeid, resp_eligible ->
respeligible, response_id -> responseid). Covers questionnaire,
questionnaire_survey, questionnaire_question, questionnaire_quest_choice,
questionnaire_question_type, and all response tables.

Update install.xml and upgrade.php (savepoints 2025111100.01-05) for
the column renames. Update backup field lists and add backward-compat
shims in restore process methods for all renamed fields.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Covers question_type, response (header + all answer types), feedback
sections and bands, and dependency records. Each persistent includes
typed define_properties() and factory methods for common queries.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…questionnaire class.

- Move classes/question/, classes/feedback/, classes/responsetype/ to classes/local/
  and update all namespace declarations and references throughout the codebase (~88 refs)
- Fix two bugs in question.php introduced by Phase 1 column renames:
  has_choices/response_table -> haschoices/responsetable in question_type lookup,
  question_id -> questionid in get_choices() query
- Add classes/questionnaire.php: typed namespaced class with property accessors,
  business logic, and capability checks, backed by persistent data layer

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Covers is_active, is_open, is_closed, is_anonymous, survey realm checks,
ownership checks, user_time_for_new_attempt (all five qtypes), user_has_saved_response,
and user_access_messages for the cases that don't require a live context.
Capability-dependent tests deferred until generator is updated for new column names.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Update all PHP, test, and form files to use renamed DB columns (typeid,
questionid, responseid, choiceid, respview, respeligible, thankspage,
thankhead, thankbody, haschoices, responsetable) throughout queries,
inserts, bulk SQL, privacy provider, and test generators.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…onventions.

- $this->type_id → $this->typeid in question.php and all subclasses
- $question->type_id → $question->typeid in all callers
- Remove resp_view/resp_eligible compat aliases from questionnaire.class.php
- $this->resp_view → $this->respview (6 occurrences)
- optional_param('type_id') → optional_param('typeid') in questions.php
- Form element 'type_id' → 'typeid' in questions_form.php; behat selector updated
- bulk_sql() alias questionid AS question_id → AS questionid; caller updated
- boolean.php bulk SQL: fix old column names (question_id, response_id, choice_id)
- Remove stale type_id compat shim from classes/questionnaire.php

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
classes/question/ → classes/local/question/ in 7 files (questions.php
and 6 test files). The require_once calls were not updated when the
question namespace was moved under local/ in Phase 2.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
question_builder() was still constructing class names with the old
\mod_questionnaire\question\ namespace instead of
\mod_questionnaire\local\question\ after the Phase 2 namespace move.
Same stale namespace in 11 assertInstanceOf calls in questiontypes_test.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
responseclass() in all question subclasses still returned
\mod_questionnaire\responsetype\ paths. Updated to
\mod_questionnaire\local\responsetype\ in 12 files:
question.php (is_subclass_of calls), text, rate, essay, drop, yesno,
numerical, date, file, check, radio, slider.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…r.php

Create mod_questionnaire\local\response\manager to centralise all response
CRUD and query operations previously scattered across questionnaire.class.php.

- New classes/local/response/manager.php with 17 methods:
  - Loading: add_user_responses, add_response, add_response_from_formdata,
    build_response_from_appdata, get_responses, user_has_saved_response
  - Saving: response_insert, response_commit, delete_insert_response,
    commit_submission_response
  - Deletion: response_delete
  - Validation: response_check_format
  - Querying: get_survey_all_responses, response_select, get_structured_response,
    get_full_submission_for_export
  - From locallib (for new callers): get_user_responses, delete_response,
    delete_responses

- questionnaire.class.php: replace each method body with a delegation call to
  responsemanager(); add responsemanager() lazy initialiser; make update_grades()
  public so the manager can call it.

locallib.php global functions are left unchanged — their callers pass questionnaire
IDs rather than objects and will be updated in a later phase.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Remove blank line after opening brace in all classes/local/db/ persistent
  records, classes/questionnaire.php, and classes/local/response/manager.php
- Add missing @var doc comment to manager.php; convert constructor property
  promotion to explicit declaration; remove decorative // --- separator lines
- Split long lines (>132 chars) in responsetype classes and mobile.php
- Fix multi-line function call formatting in questiontypes_test.php
- Remove double blank lines in db/upgrade.php
- Move questionnaire_testable class to its own file (tests/questionnaire_testable.php)
  to fix PSR1 MultipleClasses violation
- Add one-line phpdoc descriptions to all test methods in questionnaire_test.php;
  fix inline comment capitalization and punctuation throughout; convert indented
  TODO block to block comment

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The previous line-length fix used Python string escapes that
caused backslashes in namespace paths (\responsetype, \response)
to be misinterpreted as carriage returns, breaking the PHP syntax.
Replaced the broken fragments with the correct multi-line form.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace raw \$DB calls in question.php and choice.php with the
core\persistent-based record classes from classes/local/db/:

- Constructor: question_record::get_record() replaces $DB->get_record()
- get_choices(): choice_record::get_for_question() replaces $DB->get_records()
- get_dependencies(): dependency_record::get_records() replaces $DB->get_records()
- add(): new question_record()->save() replaces $DB->insert_record()
- add_choice(): new choice_record()->save() replaces $DB->insert_record()
- update_choice(): choice_record get+set+save replaces $DB->update_record()
- add_dependency(): new dependency_record()->save() replaces $DB->insert_record()
- update_dependency(): dependency_record get+set+save replaces $DB->update_record()
- delete_dependency(): dependency_record->delete() replaces $DB->delete_records()
- choice::create_from_id(): choice_record::get_record() replaces $DB->get_record()
- choice::delete_from_db_by_id(): choice_record->delete() replaces $DB->delete_records()

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Remove global $questionnaire from questionstart_survey_display() and thread
  it through the renderer → question call chain as an optional parameter
- Move dependency HTML generation from inline PHP strings to a Mustache
  template (templates/dependencylist.mustache) rendered via $this->output
- Fix sectiontext.php layering violation: replace require_once + new \questionnaire()
  with $this->questionnaire (set by question_output() before question_survey_display()
  is called); fall back gracefully when questionnaire is null

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- questionnaire.class.php: declare all DB-sourced columns (course, name, intro,
  introformat, qtype, respondenttype, respeligible, respview, notifications,
  opendate, closedate, resume, navigate, grade, sid, timemodified,
  completionsubmit, autonum, progressbar, removeafter) plus runtime properties
  ($survey, $cm, $context, $capabilities, $responses, $questionsbysec,
  $responsemanager, $rid, $usehtmleditor)
- question.php: declare $typeid, $context, $responsetype, $qid, $resultid,
  $dependquestion, $dependchoice; rename $result_id → $resultid (no underscores)
- questions_form.php: declare $moveq
- section.php, feedback_section_form.php, behat_mod_questionnaire.php:
  remove attribute (all properties already declared)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add missing \$questionnaire = null parameter to match parent signature
added in Phase 5.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add $sid, $dependquestionsand, $dependlogicand, $dependquestionsor,
  $dependlogicor to question class (no-underscore names per Moodle convention)
- Update locallib.php to use the renamed no-underscore property names
  (form field names like 'dependquestions_and' in question.php are HTML
  element names and remain unchanged)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ponses from locallib

All callers migrated to response\manager static or instance methods.
questionnaire_delete_dependencies() also removed; callers use $DB directly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- questionnaire_load_capabilities() and questionnaire_get_context()
  replaced by questionnaire::load_capabilities() instance method
- questionnaire_get_survey_list() and questionnaire_get_survey_select()
  replaced by static questionnaire::get_survey_list/select()
- settings_form.php now reads $questionnaire->capabilities directly
- All four locallib functions removed

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
questionnaire::check_page_breaks() replaces the locallib function.
All callers in questions.php (4x) and question.php (1x) updated.
locallib function removed.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ey to questionnaire class

questionnaire_set_events() → \questionnaire::set_events() (static)
questionnaire_delete_survey() → \questionnaire::delete_survey() (static)

All callers in lib.php and locallib.php updated; functions removed from locallib.php.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
pagebreak::questionstart_survey_display() was missing ': stdClass' return
type declaration and returning '' instead of new stdClass(), causing a
fatal PHP error due to incompatibility with the parent method signature.

test_user_time_daily_blocked_same_day used time() - 1800 which crosses
midnight if CI runs between 00:00 and 00:30, making the submission appear
to be yesterday. Changed to mktime(0,0,0,...) (today at midnight) which is
always the same calendar day as time() and never a future timestamp.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Three get_user_responses_for_instance() calls had multiple arguments on
one line in a multi-line call. Split each argument onto its own line.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Inside a namespaced file, bare 'stdClass' resolves to
mod_questionnaire\local\question\stdClass. Must use \stdClass
(global namespace) to match the parent method's return type.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
section.php: declare $sid and $sectionid (set by fbsections.php form data)
questionnaire.class.php: declare $strquestionnaire and $strquestionnaires
(set by complete.php for page heading strings)

PHP 8.2 deprecates dynamic property creation; these must be declared.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The constructor copies all properties from the passed \$questionnaire
object (DB record or form data) onto \$this. Form data includes standard
Moodle fields like \$description that are not questionnaire properties,
causing PHP 8.2 dynamic property deprecation warnings.

Filter with property_exists() so only declared properties are set,
preventing all future whack-a-mole dynamic property declarations.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Set by report.php and myreport.php after construction; must be declared
to avoid PHP 8.2 dynamic property deprecation.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
mchurchward and others added 26 commits May 22, 2026 14:26
…enderer/page

Convert questions.php, show_nonrespondents.php, and report.php to
construct their own \$renderer = \$PAGE->get_renderer('mod_questionnaire')
and \$page locals instead of stashing them on the questionnaire.

- questions.php passes \$renderer to both \mod_questionnaire\questions_form
  constructor sites so the form picks up the renderer it needs for
  image_url() / get_dependency_html() without falling back to
  \$questionnaire->renderer.
- show_nonrespondents.php is the largest (~55 references); all of them
  are mechanical \$questionnaire->renderer/page → \$renderer/\$page renames.
- report.php passes \$renderer and \$page through to its seven
  \$questionnaire->reporter() calls.

With this batch, every entry-point page in the plugin builds its own
renderer and page locally; the only remaining users of
\$questionnaire->renderer / ->page are the transitional fallbacks inside
reporter, questions_form, feedback_section_form, and tabs.php, which
will be removed in batch 4 alongside the questionnaire fields
themselves.

PHPUnit 353/353; Behat 56 pass / 3 known-deferred / 3 skipped.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Final 47e cleanup. With every entry-point page now constructing its own
\$renderer and \$page locals (batches 1–3) and every class-side consumer
taking those dependencies explicitly (pre-batches 91e1446 and 8a1ddad),
the renderer/page state on questionnaire is unused.

- classes/questionnaire.php drops the public \$renderer and \$page fields,
  the add_renderer() and add_page() setters, and the three transitional
  shim methods view(), print_survey() and survey_print_render() that
  delegated to the renderer classes.
- classes/reporter.php drops the constructor's \$questionnaire->renderer
  / ->page fallback; the fields stay nullable so CSV-only callers (which
  never enter render-path methods) keep working.
- classes/questions_form.php and classes/feedback_section_form.php
  promote \$renderer to a required, non-nullable constructor argument and
  drop the renderer() accessor that was bridging to \$questionnaire->renderer.
- tabs.php uses the includer's \$page directly; the \$page ??
  \$questionnaire->page fallback is gone.

questionnaire.php drops a further -88 lines on top of the earlier
Phase 47 reductions.

PHPUnit 353/353; Behat 56 pass / 3 known-deferred / 3 skipped.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Splits four `$questionnaire->reporter($renderer, $page)->method(...)`
calls in report.php and myreport.php across two lines so each line
stays within Moodle's 132-character limit.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
report.php had five raw $DB->get_record / count_records calls that fit
existing patterns: response lookups go through a new
response_record::get_or_null finder; user lookups use \core_user::get_user;
the "any complete response?" count uses the existing
response_record::count_complete_for_questionnaire helper.

Two blocks in the dvallresp and vresp arms set \$resp/\$ruser variables
that were never read in those arms; deleted as dead code.

reporter::survey_results_navbar_alpha was doing one user lookup per
response inside its build loop; switched to a single user_get_users_by_id
prefetch and an isset() check inside the loop. For the largest navbar
this drops from N+1 queries to 2.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Removed:
- questionnaire::add_survey()  — pure forwarder to survey::add_survey;
  instance_admin calls survey::add_survey directly, no other callers.
- questionnaire::copy_survey() — pure forwarder to survey::copy_survey;
  same situation.
- survey::from_record()        — zero callers anywhere in the plugin.

Downgraded to private static (only called from inside survey.php):
- survey::from_record_shallow
- survey::get_private_for_course
- survey::get_by_realm

No tests covered these surfaces directly; PHPUnit 354/354 still passing.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
survey_view_renderer::print_survey_start had three raw $DB calls:

- get_record('questionnaire_response', ['id' => $rid]) →
  response_record::get_or_null($rid)
- get_record('user', ['id' => $userid]) → \core_user::get_user($userid)
- get_record_sql joining response→questionnaire→course for a public-survey
  course name → new response_record::get_course_fullname($rid) finder

The new finder is a focused get_field_sql that returns the owning
course's fullname for a completed response (or null), covering the same
constraint set as the inlined SQL.

PHPUnit 355/355, Behat 56/3/3 matching baseline.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The last raw $DB call in capabilities.php fetched every complete
response for a (questionnaire, user) pair, sorted by submitted DESC,
then only used the first one. Replaced with a focused
response_record::get_latest_complete_for_user finder that mirrors the
existing get_latest_incomplete pattern (LIMIT 1).

PHPUnit 356/356, Behat 56/3/3 matching baseline.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
questionnaire_report_start_pdf() was a 28-line top-level function with
two callers inside report.php. Moved into a new
mod_questionnaire\output\pdf_factory::create() static so it can live
under the output namespace next to the page classes that consume it.

The TCPDF configuration is unchanged. PHPUnit 356/356, Behat 56/3/3
matching baseline. Re-ran Behat class-map enable for the new class.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The four deletion-related arms of report.php's action switch (confirm-delete
single, execute delete single, confirm-delete all, execute delete all)
shared the same capability check, survey-ownership guard, event-trigger
shape, and redirect/throw pattern. Pulled them into a new
\mod_questionnaire\report_actions class — each arm in report.php becomes
a one-line dispatch.

report.php loses ~210 lines (880 → 678 then 678 → 470 in this commit).
The remaining four arms (dwnpg, dfs, vall, vresp) and shared bootstrap
stay inline for now.

tabs.php was reading $questionnaire, $page, $currentgroupid, $rid, $USER,
$CFG from the caller's local scope; the controller's include_tabs() helper
threads each of those in explicitly so the include works the same from a
method as from the entry script.

PHPUnit 360/360 (4 new tests for the controller). Behat 56/3/3 matching
baseline. Refreshed Behat class map for the new class.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The dwnpg (download options page) and dfs (CSV/dataformat export) action
arms move out of report.php into a new \mod_questionnaire\report_downloader,
mirroring the report_actions pattern: constructor takes (questionnaire,
renderer), private include_tabs() helper threads tabs.php's caller-scope
dependencies. Each report.php arm is now a 3-line dispatch.

Adds tests/report_downloader_test.php covering capability checks on both
methods and the redirect-on-no-emails path (allowemailreporting off ->
moodle_exception under PHPUnit).

PHPUnit: 363/363 (+3). Behat: 56/3/3 baseline regression-free.
The 4 form-button branches (submit / resume / next / prev) move out of
build_survey_form() into protected methods on survey_view_renderer:
handle_submit_action, handle_resume_action, handle_next_action,
handle_prev_action. Each method owns its own validation + side effects
and returns a documented msg/null contract; build_survey_form keeps the
cascade ordering (separate if blocks) so any quirk of pressing multiple
buttons in one request is preserved.

Adds 4 per-handler tests via Reflection, covering: submit short-circuit
when SESSION->end is set, resume's savedprogress notification, next's
sec advance, and prev's walk-back from the past-end summary.

PHPUnit: 367/367 (+4). Behat: 56/3/3 baseline regression-free.
Adds three tests for the previously-uncovered "validation failed" branch
in each handler:

- handle_submit_action: required question left blank -> returns msg,
  refreshes formdata->rid.
- handle_next_action: same fixture -> returns msg, clears formdata->next,
  refreshes rid.
- handle_prev_action: date question with a non-date value -> wrongformat
  msg, clears formdata->prev, refreshes rid (prev only checks format,
  not missing-required, so the fixture uses response_valid() failure).

PHPUnit: 370/370 (+3). Behat: 56/3/3 baseline regression-free.
\core\email::__construct() now requires a stdClass for $from, so passing
$CFG->noreplyaddress (a string) trips a TypeError in CI. Switch all three
call sites in submission_notifier::send_email and savefileformat.php to
core_user::get_noreply_user(), which returns the expected user shape.

PHPUnit: 370/370.
The two remaining render arms - vall (with vallasort/vallarsort sort
variants) and vresp (with the default fall-through) - move out of
report.php into a new \mod_questionnaire\report_viewer that mirrors the
report_actions / report_downloader pattern: constructor takes
(questionnaire, renderer), private include_tabs() helper threads
tabs.php's caller-scope dependencies. report.php is now down to
entry-script housekeeping plus six 1-3 line dispatches.

Adds tests/report_viewer_test.php covering the no-permissions throw on
view_all_responses and the surveyowner throw on view_individual_response.

report.php: 560 -> 277 lines (-283).
PHPUnit: 372/372 (+2). Behat: 56/3/3 baseline regression-free.
The outer if ($usergraph) block in report.php loaded chart-rendering JS
unconditionally for every action arm, even ones that never render
charts (delete confirmations, CSV downloads, etc.). Pull the init into
a private report_viewer::init_rgraph() helper called only from
view_all_responses and view_individual_response. view_all_responses
gains a $usergraph parameter; view_individual_response's duplicate
inline block is replaced with the helper call.

report.php: 277 -> 253 lines (-24).
PHPUnit: 372/372. Behat: 56/3/3 baseline regression-free.
CLAUDE.md's unit-test policy requires direct coverage of each public
method on the responsetype hierarchy; the existing tests only exercised
these paths indirectly through create_question_response().

New tests, all driven by a small build_data_fixture() helper that
creates the course / questionnaire / question / parent response row
needed by the data-bearing methods:

- boolean::insert_response writes the questionnaire_response_bool row
- boolean::get_results returns counts grouped by choice id
- boolean::display_results returns the templatable tags object
- text::insert_response writes the questionnaire_response_text row
- text::get_results returns the inserted text rows joined to user

PHPUnit: 377/377 (+5).
Continues the responsetype direct-test pass started in 5bb11c2. Extends
build_data_fixture() with an optional choicedata parameter so single
(and future choice-based types) can reuse the helper.

New tests:

- date::insert_response writes a YYYY-MM-DD value
- date::insert_response rejects bad format and writes nothing
- date::get_results returns the stored dates
- single::insert_response writes the chosen questionnaire_resp_single row
- single::get_results joins choices so result rows carry choice content

PHPUnit: 382/382 (+5).
CI phpcs flagged "Phpdocs for function responsetype_test::build_data_fixture
has incomplete parameters list" after 390d0d5 added the optional
choicedata argument without updating the docblock.
…ted persisting)

feedback_section_form.php:156 and fbsections.php:281 both read
$questionnaire->questions()[$qid]->name as a property, but $name is a
method on the question class. The warning surfaced only after the
feedback save path started persisting correctly — until then the form
was never reached.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…llowlist

The legacy save path on feedback.php / qsettings.php built a free-form
stdClass that mixed survey persistent fields, form metadata (hidden
'id' = course-module id) and editor draft-area shapes, then handed the
whole bag to survey::update_survey(). update_survey() called
$persistent->from_record($sdata), which (since Moodle's base persistent
includes 'id' in properties_definition) overwrote the survey row's id
with the cmid. The subsequent update() then ran
UPDATE questionnaire_survey ... WHERE id = cmid, silently affecting zero
rows or — worse — mutating an unrelated survey row whose id happened to
match. feedbacksections / feedbackscores / feedbacknotes / charttype
never landed on the real survey. This was the root cause of the three
previously-deferred Behat scenarios (add_feedback,
add_multi_feedback_with_sections, slider_feedback_question_type).

This commit replaces the pattern with:

* survey::update_settings(array $fields). Only keys in the explicit
  UPDATABLE_FIELDS allowlist are accepted; anything else throws a
  coding_exception. Name/title/realm validation runs only when those
  keys are actually supplied. update_survey(stdClass) is dropped.

* Two save controllers replace the inline save blocks in the entry
  scripts and own the form->persistent marshalling:

    - feedback_settings_controller: save() handles the feedbacknotes
      draft area, picks chart_type_global / _two_sections / _sections
      when usergraph is on, and calls survey::update_settings.
      ensure_first_section() creates the initial Global Feedback section
      when the user clicks "Save settings and edit Feedback Sections"
      and no section exists yet.
    - survey_settings_controller: save() handles the info + thankbody
      draft areas and writes the qsettings allowlist.

* tests/generator/lib.php::create_content() switched from passing the
  full survey row through update_survey() to handing $record straight
  to update_settings(). All existing callers already pass only
  allowlisted fields.

Tests:
* survey_test: +5 update_settings tests (allowlisted persistence,
  unknown field rejection, form metadata rejection, empty required
  rejection, duplicate name rejection, unsupplied-field isolation).
* feedback_settings_controller_test: 6 tests covering allowlisted save,
  reset-to-zero, chart_type_global pick, chart_type_sections pick,
  ensure_first_section creates section, ensure_first_section no-op.
* survey_settings_controller_test: 3 tests covering form-field save,
  rejection on bad input, and id-corruption hardening.

Behat: full mod_questionnaire suite green — 62 scenarios, 59 passed +
3 skipped. The three previously-deferred feedback scenarios now pass.
PHPUnit: 397/397.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…tput

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@lucaboesch
lucaboesch force-pushed the REFACTOR_500_V2_codechecker branch from 7547500 to bb10ce0 Compare June 19, 2026 21:37
@lucaboesch
lucaboesch force-pushed the REFACTOR_500_V2_codechecker branch from bb10ce0 to 4439b40 Compare June 19, 2026 21:45
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