Add gala achievement #3789
Add gala achievement #3789
Conversation
aleksandervestlund
commented
Mar 17, 2025
•
aleksandervestlund
commented
- Primarily developed by @Karolipb.
- Add gala achievements to encourage attending them.
- The numbers for the different levels are very loose.
- At this moment in time, all galas count, but it could also just be "studentgallaer".
- Have no idea if it works.
- See frontend PR Add gala achievement lego-webapp#5447.
|
I like the initiative but I have some concerns. **Not saying it's a bad idea just saying there should probably be some considerations around this topic first. I will bring it up on the meeting on wednesday to maybe get some opinions. Also if you (both) have any thoughts on it please share :) |
|
Also as described in the guide, please add it to raritylist :) **And run makemigrations though if you havent set it up I can do it later its not a huge deal |
|
You make some valid points, but I feel that many members not in Abakom attend the different galas (maybe except for Vaargalla)—especially the itDAGENE banquet, Julebord and the different jubilees. Many first graders also have Immball as their first event, so this may motivate them further to attend. I kind of agree that Halvingfest and Utmatrikuleringsfest are kind of borderline galas, but in the description the dress code is described as gala, so I wasn't sure🤷🤷 |
Imo I don't see galla as something that is only for Abakom, quite the opposite. I agree that halvingfest should not count Good job! But please call it "galla" with two Ls 😆 |
|
Agree with trophys for attending galas as a potentially nice incentive for more non-Abakom to attend them.
Unless we are looking to start using Norwegian variable names, which we only currently do for specific and difficult translations (e.g. genfors), I disagree. [1] [2] [3] |
|
To fix the spelling is really simple, but what's the conclusion? We can probably delete "Halvingfest" and "Utmatrikuleringsfest" either way since they're not "studentergalla"s |
The meeting is 16:15 today we may be able to come to a conclusion then 👍👍 |
|
After a long discussion we have decided to merge this. |
|
Glad to hear!! 🤗 |
|
Probably without halvingfest and utmatrikuleringsfest. The rarities will probably be reworked soon anyways to have less rarities. |
📝 WalkthroughWalkthroughThe changes add gala-specific functionality to the achievements system. New constants and collections for gala achievements are introduced in the constants module, and the calculation utilities are updated to include gala rarity values. A dedicated function to verify a user’s gala event count is added to the verification module. In addition, a new promotion function is implemented to handle gala-related checks, and a test case is introduced to validate gala achievement levels. These modifications integrate gala achievements into the existing promotion and achievement workflows. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant P as Promotion Module
participant A as Achievements Data
participant V as Verification Module
U->>P: Trigger check_all_promotions()
P->>P: Call check_gala_related_single_user(user)
P->>A: Retrieve GALA_ACHIEVEMENTS & GALA_IDENTIFIER
P->>V: Call check_total_galas(user, required_count)
V-->>P: Return Boolean result
P-->>U: Update achievement level based on gala check
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
jonasdeluna
force-pushed
the
gala-achievement
branch
from
314b25e to
8f4bb2c
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
lego/apps/achievements/constants.py (1)
25-25: 💡 Verification agent🧩 Analysis chain
Update the rarity list as mentioned in the PR discussion.
According to the PR discussion, the rarity list in
/utils/calculation_utils.pyshould be updated when adding new achievements.
🏁 Script executed:
#!/bin/bash # Check if gala achievements have been added to the rarity list grep -n "gala" lego/utils/calculation_utils.pyLength of output: 111
I'll run an additional script to search for the
calculation_utils.pyfile anywhere in the repository to verify its correct path. This will help clarify if the file path referenced in the comment needs updating.
🏁 Script executed:
#!/bin/bash # Search for the file 'calculation_utils.py' anywhere in the repository fd "calculation_utils.py"Length of output: 75
Action Required: Correct the File Path for Rarity List Update
- The comment in
lego/apps/achievements/constants.py(line 25) incorrectly references the rarity list file as/utils/calculation_utils.py.- The actual file is located at
lego/apps/achievements/utils/calculation_utils.py.- Please update the comment to direct developers to update the rarity list in
lego/apps/achievements/utils/calculation_utils.pywhen adding new achievements.
🧹 Nitpick comments (2)
lego/apps/achievements/verification.py (1)
140-151: Implementation looks good, but consider future circular import challenges.The function correctly implements the gala verification by checking event titles against the defined gala substrings. The circular import is properly handled.
For long-term maintainability, you might consider moving all substring constants to a shared module to avoid this pattern of importing within functions.
lego/apps/achievements/tests/tests.py (1)
255-262: Consider testing each achievement level separately.While the current test verifies the highest achievement level (15 galas), it would be more robust to test each level individually (1, 5, 10, and 15) to ensure the progression works as expected.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
lego/apps/achievements/constants.py(4 hunks)lego/apps/achievements/migrations/0007_alter_achievement_identifier.py(1 hunks)lego/apps/achievements/promotion.py(2 hunks)lego/apps/achievements/tests/tests.py(3 hunks)lego/apps/achievements/utils/calculation_utils.py(2 hunks)lego/apps/achievements/verification.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- lego/apps/achievements/promotion.py
- lego/apps/achievements/utils/calculation_utils.py
🧰 Additional context used
🧬 Code Definitions (1)
lego/apps/achievements/constants.py (1)
lego/apps/achievements/verification.py (1) (1)
check_total_galas(140-150)
🔇 Additional comments (6)
lego/apps/achievements/migrations/0007_alter_achievement_identifier.py (1)
17-31: New achievement identifier correctly added!The "gala_count" has been properly added to the list of achievement identifiers in the migration, which allows the database to recognize the new achievement type.
lego/apps/achievements/constants.py (4)
37-37: New identifier correctly defined.The GALA_IDENTIFIER constant follows the naming pattern of other achievement identifiers.
40-46: Verify these gala events match the PR discussion.Based on the PR comments, there was discussion about excluding "Halvingfest" and "Utmatrikuleringsfest" from counting towards gala achievements, which appears to be implemented correctly as they're not in this list.
The comments documenting the maximum expected counts for each event type are helpful.
Could you confirm that this list aligns with the final decision from the team meeting mentioned in the PR discussion?
242-263: Achievement levels properly defined.The implementation follows the established pattern for achievement definitions with appropriate levels (0-3) corresponding to increasing attendance counts (1, 5, 10, 15).
290-290: Achievement collection properly added to ACHIEVEMENTS dictionary.The gala achievements have been correctly integrated into the global achievements collection.
lego/apps/achievements/tests/tests.py (1)
242-292: Comprehensive test case for gala achievements.The test thoroughly checks the gala achievement functionality by:
- Creating multiple gala events with different name patterns
- Registering the user for each event
- Verifying the highest achievement level is unlocked
The test aligns with the event name patterns defined in GALA_SUBSTRINGS.
|
@Karolipb If you want the lego pin you should probably message @magnusbrecke on slack or something I think he is responsible for that as he's the leader 👍 |
