Skip to content

Conversation

@st-manu
Copy link
Contributor

@st-manu st-manu commented Nov 12, 2025

https://sakaiproject.atlassian.net/browse/SAK-52151

this change was added by mistake in sakai

Summary by CodeRabbit

  • Bug Fixes
    • ZIP downloads now use consistent English identifiers for timestamp, feedback text, and comments, ensuring deterministic file names across languages.
    • Corresponding localized labels for those ZIP metadata entries have been removed from resource bundles.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 12, 2025

Walkthrough

The pull request removes three localization keys (zip.timestamp, zip.feedback_text, zip.comments) from assignment resource bundles and updates AssignmentServiceImpl to use fixed English identifiers ("timestamp", "feedbackText", "comments") for ZIP entry names in single and grouped submission ZIP creation.

Changes

Cohort / File(s) Summary
Resource bundle removals
assignment/api/src/resources/assignment.properties, assignment/api/src/resources/assignment_ca.properties, assignment/api/src/resources/assignment_es.properties
Deleted three keys: zip.timestamp, zip.feedback_text, and zip.comments from properties files.
ZIP entry naming logic
assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java
Replaced resourceBundle lookups for ZIP metadata filenames with hard-coded English identifiers ("timestamp", "feedbackText", "comments") for both per-submission and group ZIP creation paths; no data content changes.

Possibly related PRs

Suggested labels

I18N

Suggested reviewers

  • ottenhoff
  • juanjmerono

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Title check ⚠️ Warning The PR title claims to add Spanish and Catalan translations, but the changes actually remove translation keys and replace localized strings with hard-coded English identifiers, which is the opposite of the stated objective. Update the title to accurately reflect that the PR removes localization and hard-codes English identifiers (e.g., 'SAK-52151 Remove localized ZIP entry names and use hard-coded English identifiers').
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f9584af and 4b27b6c.

📒 Files selected for processing (4)
  • assignment/api/src/resources/assignment.properties (0 hunks)
  • assignment/api/src/resources/assignment_ca.properties (0 hunks)
  • assignment/api/src/resources/assignment_es.properties (0 hunks)
  • assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java (7 hunks)
💤 Files with no reviewable changes (3)
  • assignment/api/src/resources/assignment_es.properties
  • assignment/api/src/resources/assignment.properties
  • assignment/api/src/resources/assignment_ca.properties
🧰 Additional context used
📓 Path-based instructions (1)
**/*.java

📄 CodeRabbit inference engine (.cursor/rules/logging-rule.mdc)

**/*.java: Use SLF4J parameterized logging (logger.info("Value is: {}", value)) instead of string concatenation (logger.info("Value is: " + value))
Log messages and code comments should be in English. Log messages should never be translated.

**/*.java: Java: Never use local variable type inference (var). Always declare explicit types. Yes: Map<String, Integer> counts = new HashMap<>(); No: var counts = new HashMap<String, Integer>();
When proposing Java code, spell out full types in local variable declarations, for loops, and try-with-resources
When editing Java, prefer clarity over brevity; avoid introducing language features that aren't widely used in the repo
Treat any PR or suggestion containing Java var as non-compliant. Recommend replacing with explicit types before merge

**/*.java: Use Java 17 for trunk development (Java 11 was used for Sakai 22 and Sakai 23)
Do not use local variable type inference (var) in Java code. Always declare explicit types (e.g., List<String> names = new ArrayList<>(); not var names = new ArrayList<String>();). Enforced by Checkstyle rule during mvn validate

Files:

  • assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: sakaiproject/sakai PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T19:12:27.663Z
Learning: Applies to **/sakai-notifications.properties : Reconcile subscriptions proactively and keep localization strings in `sakai-notifications.properties`
📚 Learning: 2025-11-24T19:12:27.663Z
Learnt from: CR
Repo: sakaiproject/sakai PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T19:12:27.663Z
Learning: Applies to **/sakai-notifications.properties : Reconcile subscriptions proactively and keep localization strings in `sakai-notifications.properties`

Applied to files:

  • assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java
📚 Learning: 2025-10-07T15:11:27.298Z
Learnt from: ottenhoff
Repo: sakaiproject/sakai PR: 0
File: :0-0
Timestamp: 2025-10-07T15:11:27.298Z
Learning: In samigo’s Total Scores view (samigo/samigo-app/src/webapp/jsf/evaluation/totalScores.jsp), mailto links were hidden after commit dee05746 (PR #12312, SAK-49674) added a render check requiring email.fromEmailAddress to be non-empty; PR #14154 (SAK-52058) restores visibility by checking only description.email.

Applied to files:

  • assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java
📚 Learning: 2025-10-07T16:11:33.008Z
Learnt from: CR
Repo: sakaiproject/sakai PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-07T16:11:33.008Z
Learning: Commit messages should follow: <issue key> <component> <brief description> (e.g., SAK-12345 Assignments add option x)

Applied to files:

  • assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java
📚 Learning: 2025-11-24T19:12:27.663Z
Learnt from: CR
Repo: sakaiproject/sakai PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T19:12:27.663Z
Learning: Define custom HTML elements for Sakai-specific functionality

Applied to files:

  • assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java
📚 Learning: 2025-11-24T19:12:20.537Z
Learnt from: CR
Repo: sakaiproject/sakai PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T19:12:20.537Z
Learning: Format commit messages as `<issue key> <component> <brief description>` (e.g., `SAK-12345 Assignments add option x`)

Applied to files:

  • assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java
📚 Learning: 2025-10-07T16:35:33.991Z
Learnt from: ern
Repo: sakaiproject/sakai PR: 14152
File: reset-pass/account-validator-impl/pom.xml:37-42
Timestamp: 2025-10-07T16:35:33.991Z
Learning: The Sakai project cannot upgrade to Spring Framework 6 yet and must remain on Spring 5.x for the time being.

Applied to files:

  • assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java
📚 Learning: 2025-10-06T22:15:40.541Z
Learnt from: ottenhoff
Repo: sakaiproject/sakai PR: 14152
File: reset-pass/account-validator-api/src/main/java/org/sakaiproject/accountvalidator/model/ValidationAccount.java:24-33
Timestamp: 2025-10-06T22:15:40.541Z
Learning: The Sakai project (as of this PR) uses Hibernate 5.6.15.Final and Spring 5.3.39, which requires javax.persistence imports, not jakarta.persistence. The jakarta namespace migration will happen when the project upgrades to JDK 21, Spring 6, and Hibernate 6+.

Applied to files:

  • assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java
🧬 Code graph analysis (1)
assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java (1)
assignment/api/src/java/org/sakaiproject/assignment/api/AssignmentConstants.java (1)
  • AssignmentConstants (31-395)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: maven-build
  • GitHub Check: sakai-deploy
  • GitHub Check: maven-build
🔇 Additional comments (2)
assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java (2)

3651-3655: Stable, locale‑independent timestamp filenames look good

Using the fixed "timestamp" basename (combined with ZIP_COMMENT_FILE_TYPE) yields deterministic, ASCII‑only ZIP entry names independent of locale, while remaining unique per submitter/group folder. No issues spotted.

Also applies to: 3907-3910


3673-3679: Consistent feedbackText ZIP entry naming is fine

Hard‑coding "feedbackText" as the basename for feedback text entries (with ZIP_SUBMITTED_TEXT_FILE_TYPE) gives clear, stable filenames across locales and mirrors the timestamp approach without changing content semantics. Looks correct.

Also applies to: 3920-3924

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ast-grep (0.40.0)
assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java

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.

Copy link
Contributor

@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

🧹 Nitpick comments (1)
assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java (1)

3702-3707: Stable ZIP entry name for comments — LGTM (consider constants)

Works as intended. Consider centralizing basenames as constants to avoid drift and ease reuse.

- final String zipEntryName = submittersName + "comments" + AssignmentConstants.ZIP_COMMENT_FILE_TYPE;
+ final String zipEntryName = submittersName + AssignmentConstants.ZIP_COMMENTS_BASENAME + AssignmentConstants.ZIP_COMMENT_FILE_TYPE;

And in AssignmentConstants.java:

+ public static final String ZIP_TIMESTAMP_BASENAME = "timestamp";
+ public static final String ZIP_FEEDBACK_TEXT_BASENAME = "feedbackText";
+ public static final String ZIP_COMMENTS_BASENAME = "comments";
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a43b995 and f9584af.

📒 Files selected for processing (2)
  • assignment/api/src/resources/assignment.properties (0 hunks)
  • assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java (7 hunks)
💤 Files with no reviewable changes (1)
  • assignment/api/src/resources/assignment.properties
🧰 Additional context used
📓 Path-based instructions (1)
**/*.java

📄 CodeRabbit inference engine (.cursor/rules/logging-rule.mdc)

**/*.java: Use SLF4J parameterized logging (e.g., logger.info("Value is: {}", value)) instead of string concatenation in log statements
Write log messages and code comments in English; never translate log messages

**/*.java: Java: Never use local variable type inference (var). Always declare explicit types.
When proposing or writing Java code, spell out full types in local variable declarations, for-loops, and try-with-resources.
If Java code uses var, replace it with an explicit type.
Review gate: Treat any PR or suggestion containing Java var as non-compliant; require explicit types before merge.

Files:

  • assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java
🧬 Code graph analysis (1)
assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java (1)
assignment/api/src/java/org/sakaiproject/assignment/api/AssignmentConstants.java (1)
  • AssignmentConstants (31-395)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: sakai-deploy
  • GitHub Check: maven-build
  • GitHub Check: maven-build
🔇 Additional comments (3)
assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java (3)

3673-3679: Stable ZIP entry name for feedback text — LGTM

"feedbackText" basename looks good and consistent.


3907-3910: Group ZIP: stable timestamp entry — LGTM

Consistent with the single‑submission flow.


3921-3924: Group ZIP: stable feedback text entry — LGTM

Matches the non‑group naming.

@jesusmmp jesusmmp merged commit f7ae398 into sakaiproject:master Dec 3, 2025
5 checks passed
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