Skip to content

fixed lang error#1628

Closed
ggoffy wants to merge 1 commit into
XOOPS:masterfrom
ggoffy:master
Closed

fixed lang error#1628
ggoffy wants to merge 1 commit into
XOOPS:masterfrom
ggoffy:master

Conversation

@ggoffy
Copy link
Copy Markdown
Contributor

@ggoffy ggoffy commented Feb 24, 2026

Summary by CodeRabbit

  • Refactor
    • Reorganized language strings related to user access management in the admin system module for improved message consistency.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Feb 24, 2026

Walkthrough

This change reorganizes language constants in the English admin modules language file, consolidating user-access message definitions. The constant _AM_SYSTEM_MODULES_ACCESS_USER_ADD_ERROR_ERROR is removed, while _AM_SYSTEM_MODULES_ACCESS_USER_ADD is introduced for success messages and _AM_SYSTEM_MODULES_ACCESS_USER_ADD_ERROR is reassigned for error messages.

Changes

Cohort / File(s) Summary
Language Constants Consolidation
htdocs/modules/system/language/english/admin/modulesadmin.php
Renamed and reassigned user-access constants: removed redundant _ADD_ERROR_ERROR, created _ADD for success message, reassigned _ADD_ERROR for error message. Fixes inconsistent naming pattern.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related PRs

Suggested reviewers

  • Copilot
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'fixed lang error' is vague and generic, using non-descriptive terminology that doesn't convey the specific nature of the language constant reorganization. Provide a more specific title like 'Reorganize user access language constants' or 'Fix user access group message constant naming' to clearly describe the constant consolidation changes.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown

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

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@htdocs/modules/system/language/english/admin/modulesadmin.php`:
- Around line 136-139: The user-facing strings are inconsistent:
_AM_SYSTEM_MODULES_ACCESS_ADMIN_ADD and
_AM_SYSTEM_MODULES_ACCESS_ADMIN_ADD_ERROR use "Group ID %s" (no colon) while
_AM_SYSTEM_MODULES_ACCESS_USER_ADD and _AM_SYSTEM_MODULES_ACCESS_USER_ADD_ERROR
use "Group ID: %s"; update the two user constants
(_AM_SYSTEM_MODULES_ACCESS_USER_ADD and
_AM_SYSTEM_MODULES_ACCESS_USER_ADD_ERROR) to use "Group ID %s" (remove the
colon) so all four messages follow the same format.
- Around line 138-139: The two new language constants
_AM_SYSTEM_MODULES_ACCESS_USER_ADD and _AM_SYSTEM_MODULES_ACCESS_USER_ADD_ERROR
use "Group ID: %s" with a colon while the admin counterparts use "Group ID %s"
without a colon; update these two constants to remove the colon so they read
"Group ID %s" to keep punctuation consistent with the admin messages, and run a
quick grep for _AM_SYSTEM_MODULES_ACCESS_* to ensure all related message pairs
use the same placeholder format.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b6302f4 and 5a0b0a9.

📒 Files selected for processing (1)
  • htdocs/modules/system/language/english/admin/modulesadmin.php

Comment on lines 136 to +139
define('_AM_SYSTEM_MODULES_ACCESS_ADMIN_ADD', 'Added admin access right for Group ID %s');
define('_AM_SYSTEM_MODULES_ACCESS_ADMIN_ADD_ERROR', 'ERROR: Could not add admin access right for Group ID %s');
define('_AM_SYSTEM_MODULES_ACCESS_USER_ADD_ERROR', 'Added user access right for Group ID: %s');
define('_AM_SYSTEM_MODULES_ACCESS_USER_ADD_ERROR_ERROR', 'ERROR: Could not add user access right for Group ID: %s');
define('_AM_SYSTEM_MODULES_ACCESS_USER_ADD', 'Added user access right for Group ID: %s');
define('_AM_SYSTEM_MODULES_ACCESS_USER_ADD_ERROR', 'ERROR: Could not add user access right for Group ID: %s');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Inconsistent "Group ID" format between admin and user access messages.

Lines 136–137 (admin constants) format the placeholder as Group ID %s (no colon), while lines 138–139 (user constants) use Group ID: %s (with colon). Pick one and apply it consistently.

🔧 Proposed fix — align with the existing admin format (no colon)
-define('_AM_SYSTEM_MODULES_ACCESS_USER_ADD', 'Added user access right for Group ID: %s');
-define('_AM_SYSTEM_MODULES_ACCESS_USER_ADD_ERROR', 'ERROR: Could not add user access right for Group ID: %s');
+define('_AM_SYSTEM_MODULES_ACCESS_USER_ADD', 'Added user access right for Group ID %s');
+define('_AM_SYSTEM_MODULES_ACCESS_USER_ADD_ERROR', 'ERROR: Could not add user access right for Group ID %s');
📝 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
define('_AM_SYSTEM_MODULES_ACCESS_ADMIN_ADD', 'Added admin access right for Group ID %s');
define('_AM_SYSTEM_MODULES_ACCESS_ADMIN_ADD_ERROR', 'ERROR: Could not add admin access right for Group ID %s');
define('_AM_SYSTEM_MODULES_ACCESS_USER_ADD_ERROR', 'Added user access right for Group ID: %s');
define('_AM_SYSTEM_MODULES_ACCESS_USER_ADD_ERROR_ERROR', 'ERROR: Could not add user access right for Group ID: %s');
define('_AM_SYSTEM_MODULES_ACCESS_USER_ADD', 'Added user access right for Group ID: %s');
define('_AM_SYSTEM_MODULES_ACCESS_USER_ADD_ERROR', 'ERROR: Could not add user access right for Group ID: %s');
define('_AM_SYSTEM_MODULES_ACCESS_ADMIN_ADD', 'Added admin access right for Group ID %s');
define('_AM_SYSTEM_MODULES_ACCESS_ADMIN_ADD_ERROR', 'ERROR: Could not add admin access right for Group ID %s');
define('_AM_SYSTEM_MODULES_ACCESS_USER_ADD', 'Added user access right for Group ID %s');
define('_AM_SYSTEM_MODULES_ACCESS_USER_ADD_ERROR', 'ERROR: Could not add user access right for Group ID %s');
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@htdocs/modules/system/language/english/admin/modulesadmin.php` around lines
136 - 139, The user-facing strings are inconsistent:
_AM_SYSTEM_MODULES_ACCESS_ADMIN_ADD and
_AM_SYSTEM_MODULES_ACCESS_ADMIN_ADD_ERROR use "Group ID %s" (no colon) while
_AM_SYSTEM_MODULES_ACCESS_USER_ADD and _AM_SYSTEM_MODULES_ACCESS_USER_ADD_ERROR
use "Group ID: %s"; update the two user constants
(_AM_SYSTEM_MODULES_ACCESS_USER_ADD and
_AM_SYSTEM_MODULES_ACCESS_USER_ADD_ERROR) to use "Group ID %s" (remove the
colon) so all four messages follow the same format.

Comment on lines +138 to +139
define('_AM_SYSTEM_MODULES_ACCESS_USER_ADD', 'Added user access right for Group ID: %s');
define('_AM_SYSTEM_MODULES_ACCESS_USER_ADD_ERROR', 'ERROR: Could not add user access right for Group ID: %s');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for all references to the user-access constants

echo "=== References to new _AM_SYSTEM_MODULES_ACCESS_USER_ADD (success) ==="
rg -rn --type php '_AM_SYSTEM_MODULES_ACCESS_USER_ADD[^_]' || echo "No matches found"

echo ""
echo "=== References to _AM_SYSTEM_MODULES_ACCESS_USER_ADD_ERROR (repurposed) ==="
rg -rn --type php '_AM_SYSTEM_MODULES_ACCESS_USER_ADD_ERROR' || echo "No matches found"

echo ""
echo "=== References to removed _AM_SYSTEM_MODULES_ACCESS_USER_ADD_ERROR_ERROR ==="
rg -rn --type php '_AM_SYSTEM_MODULES_ACCESS_USER_ADD_ERROR_ERROR' || echo "No matches found"

echo ""
echo "=== Context: Definition file location ==="
cat -n htdocs/modules/system/language/english/admin/modulesadmin.php | sed -n '136,142p'

Repository: XOOPS/XoopsCore25

Length of output: 1786


Format consistency: Use uniform punctuation for message placeholders.

The new constants at lines 138–139 have a colon after "Group ID" (Group ID: %s), but the admin equivalents at lines 136–137 omit it (Group ID %s). Choose one style and apply it consistently across both admin and user message sets.

Note: All call sites already correctly reference these constants with their intended semantic meaning (success vs. error), so the constant refactoring is coordinated.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@htdocs/modules/system/language/english/admin/modulesadmin.php` around lines
138 - 139, The two new language constants _AM_SYSTEM_MODULES_ACCESS_USER_ADD and
_AM_SYSTEM_MODULES_ACCESS_USER_ADD_ERROR use "Group ID: %s" with a colon while
the admin counterparts use "Group ID %s" without a colon; update these two
constants to remove the colon so they read "Group ID %s" to keep punctuation
consistent with the admin messages, and run a quick grep for
_AM_SYSTEM_MODULES_ACCESS_* to ensure all related message pairs use the same
placeholder format.

@mambax7
Copy link
Copy Markdown
Collaborator

mambax7 commented Feb 24, 2026

This has been already fixed, see: #1621

@mambax7 mambax7 closed this Feb 24, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (master@c0674e2). Learn more about missing BASE report.

Additional details and impacted files
@@           Coverage Diff            @@
##             master   #1628   +/-   ##
========================================
  Coverage          ?       0           
========================================
  Files             ?       0           
  Lines             ?       0           
  Branches          ?       0           
========================================
  Hits              ?       0           
  Misses            ?       0           
  Partials          ?       0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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