features adding hs credit license to the teacher #7931
Conversation
WalkthroughThis change introduces support for a new course and license type called "AI HackStack" across the application. It adds the Changes
Sequence Diagram(s)sequenceDiagram
participant AdminUser as Admin User
participant AdminModal as AdministerUserModal
participant Backend as Backend (Prepaid, Schemas)
participant UI as UI Templates
AdminUser->>AdminModal: Selects "HackStack" license type and enters credit details
AdminModal->>AdminModal: Parses credit details and sets isHackstack flag
AdminModal->>Backend: Creates Prepaid license with creditDetails for HACKSTACK
Backend->>Backend: Stores license with creditDetails in Prepaid schema
Backend-->>AdminModal: Returns created license
AdminModal->>UI: Passes license data
UI->>UI: Displays HackStack license and credits using localization
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
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.31.1)app/locale/en.jsTip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
app/templates/teachers/common/license-type-mixin.pug (2)
30-44: Comprehensive credit configuration UI.The credit configuration section provides a complete set of inputs for configuring hackstack credits:
- Duration amount (numeric input)
- Duration unit (day/week/month dropdown)
- Credit limit (numeric input)
The UI is conditionally displayed only when the hackstack license type is selected, and pre-populates values from the credit object.
Suggestion: Consider adding min/max validation for the credit limit and duration amount to prevent unreasonable values.
- <input(type="number", min=1, name="durationAmount", value=credit.amount)> + <input(type="number", min=1, max=365, name="durationAmount", value=credit.amount)> - <input(type="number", min=1, name='creditLimit', value=credit.limit)> + <input(type="number", min=1, max=10000, name='creditLimit', value=credit.limit)>
32-44: Improve text label accessibility.The static text elements ("Every" and "prompts") should be more descriptive or have appropriate ARIA labels to improve accessibility.
- | Every + span(aria-hidden="true") Every + span.sr-only Duration: Credits refresh every - | prompts + span(aria-hidden="true") prompts + span.sr-only Credit limit: Maximum number of prompts per durationapp/locale/en.js (1)
4533-4533: Minor capitalization inconsistency noted.There's a slight inconsistency in the capitalization between "HackStack" in the teacher namespace (line 2737) and "Hackstack" here in the admin namespace. Consider standardizing the capitalization across all instances for brand consistency.
- license_type_hackstack: 'Hackstack Credits', + license_type_hackstack: 'HackStack Credits',app/views/admin/AdministerUserModal.js (3)
303-312: Consider adding validation for HackStack credit parameters.The implementation correctly handles the new HackStack license type by setting the appropriate course ID and collecting credit-related parameters. However, there's no validation to ensure positive values for
durationAmountorlimit.if (attrs.licenseType === 'hackstack') { attrs.includedCourseIDs = [utils.courseIDs.HACKSTACK] isHackstack = true hackstackCredits.durationAmount = parseInt(attrs.durationAmount) + if (isNaN(hackstackCredits.durationAmount) || hackstackCredits.durationAmount <= 0) { + alert('Duration amount must be a positive number') + return + } hackstackCredits.durationKey = attrs.durationKey hackstackCredits.limit = parseInt(attrs.creditLimit) + if (isNaN(hackstackCredits.limit) || hackstackCredits.limit <= 0) { + alert('Credit limit must be a positive number') + return + } delete attrs.durationAmount delete attrs.durationKey delete attrs.creditLimit }🧰 Tools
🪛 Biome (1.9.4)
[error] 309-309: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 310-310: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 311-311: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
309-311: Consider using undefined assignment instead of delete operator.The
deleteoperator can have a slight performance impact. Consider using undefined assignment instead.- delete attrs.durationAmount - delete attrs.durationKey - delete attrs.creditLimit + attrs.durationAmount = undefined + attrs.durationKey = undefined + attrs.creditLimit = undefined🧰 Tools
🪛 Biome (1.9.4)
[error] 309-309: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 310-310: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 311-311: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
323-326: Small optimization: Unnecessary object reset.Resetting
hackstackCredits = {}after assignment is unnecessary since the object is no longer used.if (isHackstack) { attrs.properties.creditDetails = hackstackCredits - hackstackCredits = {} }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
app/core/utils.js(3 hunks)app/locale/en.js(2 hunks)app/models/Prepaid.js(1 hunks)app/schemas/models/prepaid.schema.js(1 hunks)app/schemas/models/user_credit.schema.js(1 hunks)app/templates/admin/administer-user-modal.pug(1 hunks)app/templates/courses/enrollments-view.pug(1 hunks)app/templates/teachers/common/license-type-mixin.pug(3 hunks)app/views/admin/AdministerUserModal.js(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
app/core/utils.js (1)
app/models/User.js (1)
courseIDs(677-677)
🪛 Biome (1.9.4)
app/views/admin/AdministerUserModal.js
[error] 309-309: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 310-310: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 311-311: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🔇 Additional comments (16)
app/core/utils.js (2)
290-291: LGTM: HACKSTACK course added to orderedCourseIDsThe HACKSTACK course has been properly added to the end of the orderedCourseIDs array for CodeCombat, ensuring it will be included in course ordering and references.
361-362: LGTM: HACKSTACK course added to otherOrderedCourseIDsThe HACKSTACK course ID has been properly added to the end of the otherOrderedCourseIDs array for Ozaria, maintaining consistency between products.
app/templates/admin/administer-user-modal.pug (1)
304-304: LGTM: Credit details parameter added to license-type mixinThe view.creditDetails parameter has been added to the license-type mixin call, enabling support for the new credit-based HackStack license type.
app/schemas/models/prepaid.schema.js (1)
34-42: LGTM: Credit details schema added for HACKSTACK license typeThe creditDetails schema has been added with appropriate properties for tracking duration and limits for HackStack licenses:
- durationAmount: numeric value for time period
- durationKey: enumerated string ('day', 'week', 'month') for time unit
- limit: numeric value for maximum credits
This schema will allow proper validation of credit details for HackStack licenses.
app/schemas/models/user_credit.schema.js (1)
30-39: LGTM: License credits tracking added to UserCredit schemaA licenses array has been added to the UserCredit schema to track credit-based license information, with fields that align with the prepaid schema's creditDetails:
- operation: likely indicates the credit action type
- durationKey: time unit matching prepaid schema
- durationAmount: time period matching prepaid schema
- limit: credit limit matching prepaid schema
This addition enables proper tracking of HackStack license credits at the user level.
app/templates/courses/enrollments-view.pug (4)
109-110: Well-structured license detection logic.The code correctly identifies hackstack licenses by checking three conditions:
- Presence of creditDetails in properties
- Exactly one includedCourseID
- That ID matches the HACKSTACK course ID
This approach ensures proper license type identification.
113-114: Proper internationalization for hackstack license display.The code appropriately uses the i18n system for displaying the hackstack license label, maintaining consistency with how other license types are displayed.
120-123: Clear hackstack credits display with localization.The code effectively handles the special case for hackstack licenses by displaying credits information instead of course acronyms, using proper localization by passing the credit object to the translation function.
124-125: Fallback logic for non-hackstack licenses.The else branch correctly maintains the existing behavior for regular licenses, ensuring backward compatibility.
app/models/Prepaid.js (1)
80-83: Good use of optional chaining and license type differentiation.The implementation correctly:
- Uses optional chaining (
?.) to safely access nested properties- Checks for the presence of credit details and the HACKSTACK course ID
- Returns appropriate localized text by combining the license type and credit details
This maintains consistency with the UI changes in the templates while ensuring the model layer properly supports the new license type.
app/templates/teachers/common/license-type-mixin.pug (2)
1-1: Properly updated mixin signature.The mixin signature has been updated to include the new
creditparameter, which is essential for handling the hackstack license type configuration.
11-14: Clear hackstack license option with i18n support.The new radio button option for hackstack licenses is properly implemented and uses the i18n system for displaying the label, maintaining consistency with other license types.
app/locale/en.js (1)
2737-2738: Localization strings for HackStack license added correctly.These new localization entries align with the PR objective of adding high school credit license functionality for teachers. The placeholder structure in
hackstack_creditsallows for dynamic insertion of credit limits and duration information.app/views/admin/AdministerUserModal.js (3)
132-136: LGTM: New credit defaults look good.The addition of the
creditDetailsobject with sensible defaults will support the new HackStack license type.
292-294: LGTM: Clean variable declarations.The new variables for tracking HackStack license type and its credit details are clearly defined.
303-326:✅ Verification successful
Verify field names match between frontend and backend.
Ensure that the field names used here (
durationAmount,durationKey,limit) match what the backend expects increditDetails.
🏁 Script executed:
#!/bin/bash # Look for references to creditDetails structure in schema files rg -A 10 -p "creditDetails" --glob "*.schema.js" .Length of output: 494
Field Name Mapping Verified: All field names correctly align between the frontend and backend.
- The backend schema (
app/schemas/models/prepaid.schema.js) defines thecreditDetailsobject with propertiesdurationAmount,durationKey, andlimit.- The frontend code correctly maps the UI fields by:
- Parsing and assigning
attrs.durationAmounttohackstackCredits.durationAmount,- Passing
attrs.durationKeydirectly tohackstackCredits.durationKey,- Converting
attrs.creditLimitto an integer and assigning it tohackstackCredits.limit.- This mapping ensures that the backend receives the expected structure.
🧰 Tools
🪛 Biome (1.9.4)
[error] 309-309: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 310-310: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 311-311: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 314-314: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🪛 ESLint
[error] 320-321: Missing trailing comma.
(comma-dangle)
[error] 321-322: Missing trailing comma.
(comma-dangle)
…-hs-credits-feature
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
app/schemas/models/user_credit.schema.js (1)
30-41: Schema properly extended to support Hackstack license credits.The addition of the
licensesarray to theUserCreditSchemaeffectively supports credit-based licensing for the new Hackstack course. The schema includes all necessary fields for tracking license credits:prepaidId,endDate,operation,durationKey,durationAmount, andlimit.Adding
endDateis appropriate as previously discussed in past review comments.
🧹 Nitpick comments (1)
app/views/admin/AdministerUserModal.js (1)
292-317: Properly implemented Hackstack license type support.The implementation correctly handles the Hackstack license type, including setting the appropriate course IDs and capturing credit parameters from the form. However, there's a performance concern with using the
deleteoperator.Consider using undefined assignment instead of delete for better performance:
- delete attrs.durationAmount - delete attrs.durationKey - delete attrs.creditLimit + attrs.durationAmount = undefined + attrs.durationKey = undefined + attrs.creditLimit = undefined🧰 Tools
🪛 Biome (1.9.4)
[error] 300-300: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 314-314: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 315-315: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 316-316: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/core/utils.js(3 hunks)app/locale/en.js(2 hunks)app/schemas/models/prepaid.schema.js(1 hunks)app/schemas/models/user_credit.schema.js(1 hunks)app/templates/admin/administer-user-modal.pug(1 hunks)app/views/admin/AdministerUserModal.js(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- app/templates/admin/administer-user-modal.pug
- app/schemas/models/prepaid.schema.js
- app/core/utils.js
- app/locale/en.js
🧰 Additional context used
🪛 Biome (1.9.4)
app/views/admin/AdministerUserModal.js
[error] 314-314: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 315-315: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 316-316: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: ESLint CI
- GitHub Check: Node.js CI (18.10.0)
🔇 Additional comments (2)
app/views/admin/AdministerUserModal.js (2)
132-136: Good defaults for Hackstack credit configuration.The default values for
creditDetailshave been updated from using days to using weeks, and increasing the limit from 10 to 30, which is better aligned with high school credit licensing requirements.
328-331: Correctly sets creditDetails properties for Hackstack licenses.This code properly adds the credit details to the prepaid license properties when creating a Hackstack license.
fix ENG-1426
this pr is ready, but draft because we didn't implement server logic to redeem such license yet.
Summary by CodeRabbit
New Features
Localization
Enhancements