-
-
Notifications
You must be signed in to change notification settings - Fork 263
open questions admin page expanded #3894
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
A note has been added regarding open questions from other languages in the admin panel.
📝 WalkthroughWalkthroughAdds per-language question retrieval and aggregation for the admin "open questions" page, passes per-language counts and UI messages to the template, conditionally renders a warning listing other-language question counts, and adds five new translation keys in German and English. Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as Admin User
participant Controller as OpenQuestionsController
participant Question as Question Model
participant Repository as QuestionRepository
participant Template as Template Engine
participant Browser as Browser/UI
Admin->>Controller: Request open questions page
Controller->>Controller: Determine current language
Controller->>Question: getAll(showAll, currentLanguage)
Question->>Question: Resolve target language
Question->>Repository: getAll(targetLang, showAll)
Repository->>Repository: Build conditional language filter
Repository-->>Question: Return questions array
Question-->>Controller: Return questions
Controller->>Controller: Aggregate counts by language
Controller->>Template: Pass currentLocale, questions, otherCount, otherLangsData, messages
Template->>Template: Render questions table
Template->>Template: If otherCount > 0: Render warning alert with language list
Template-->>Browser: HTML response
Browser->>Admin: Display page with language alert
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@phpmyfaq/assets/templates/admin/content/open-questions.twig`:
- Around line 89-95: The template uses msgOtherQuestionDesc (singular) when
otherCount == 1 but the controller only provides msgOtherQuestionsDesc (plural);
update the controller (e.g., OpenQuestionsController, the method that builds the
template data / calls render) to also set msgOtherQuestionDesc to the correct
singular translation string (or derive it from msgOtherQuestionsDesc) before
rendering so the template has both msgOtherQuestionDesc and
msgOtherQuestionsDesc available.
In `@phpmyfaq/src/phpMyFAQ/Controller/Administration/OpenQuestionsController.php`:
- Line 47: The assigned-but-unused variable $session should be removed to
satisfy static analysis: delete the assignment using $this->container->get(id:
'session') (the $session variable) from the OpenQuestionsController code so no
unused variable remains; if session is actually needed elsewhere, instead access
it inline where used or keep a single reference in the relevant method (search
for $session and OpenQuestionsController to locate the code).
- Around line 97-100: In OpenQuestionsController, the view data array is missing
the singular translation key used by the template; add 'msgOtherQuestionDesc' =>
Translation::get(key: 'msgOtherQuestionDesc') alongside the existing
'msgOtherQuestionsDesc' entry so the template can correctly display the singular
case (otherCount == 1); update the data passed from the method that prepares
these translations in OpenQuestionsController to include this new key.
In `@phpmyfaq/src/phpMyFAQ/Question/QuestionRepository.php`:
- Around line 27-30: The constructor formatting in class QuestionRepository (the
__construct method with the private Configuration $configuration parameter)
violates the project's mago format rules; run the project's formatter (mago
format) on phpMyFAQ/Question/QuestionRepository.php or manually adjust the
constructor declaration to match project style (fix parameter spacing/commas and
indentation) and save so the mago format check passes.
phpmyfaq/src/phpMyFAQ/Controller/Administration/OpenQuestionsController.php
Outdated
Show resolved
Hide resolved
phpmyfaq/src/phpMyFAQ/Controller/Administration/OpenQuestionsController.php
Show resolved
Hide resolved
There was a problem hiding this 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
🤖 Fix all issues with AI agents
In `@phpmyfaq/src/phpMyFAQ/Question/QuestionRepository.php`:
- Around line 125-130: The getAll method's behavior when $language is an empty
string currently treats that as "all languages" via $langFilter, which
contradicts the existing PHPDoc that implies a required language; update the
PHPDoc for QuestionRepository::getAll (and mention $language param) to clearly
state that passing an empty string will return questions from all languages, or
alternatively enforce a non-empty $language by validating $language and throwing
or normalizing it; pick one approach and make the doc and implementation
consistent (refer to getAll, $language, and $langFilter).
A note has been added regarding open questions from other languages in the admin panel.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.