Skip to content
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

feat(sponsor): add diamond level #1207

Merged
merged 1 commit into from
Aug 5, 2024
Merged

feat(sponsor): add diamond level #1207

merged 1 commit into from
Aug 5, 2024

Conversation

SivanYeh
Copy link
Collaborator

@SivanYeh SivanYeh commented Jul 27, 2024

Types of changes

Thanks for sending a pull request! Please fill in the following content to let us know better about this change.
Please put an x in the box that applies

  • Bugfix
  • New feature
  • Refactoring
  • Breaking change (any change that would cause existing functionality to not work as expected)
  • Documentation Update
  • Other (please describe)

Description

Add "Diamond" into sponsor levels in the backstage.

Steps to Test This Pull Request

Steps to reproduce the behavior:

  1. Go to path/admin/sponsors/sponsor/
  2. Check if level "Diamond"/"鑽石級" is in the list

Expected behavior

image

More Information

Discord message: https://discord.com/channels/752904426057892052/1231150103360835585/1266414865489330238

@@ -66,20 +66,22 @@ class Sponsor(ConferenceRelated):
)

class Level:
PLATINUM = 1
Copy link
Member

@mattwang44 mattwang44 Jul 28, 2024

Choose a reason for hiding this comment

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

DB 裡面存的是整數代號,對於舊資料來說這樣的改動會使其在 backend 所代表的意義不同(e.g. 某 sponsor 原為 gold 所以 sponsor.level 設定為 2,但這次改動後就變成 platinum),請思考一下該怎麼處理才能達成資料意義的一致性

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

謝謝提醒!我把數值調回去並把鑽石設為0了(發現鑽石級設為num <=0 也可以達成同樣功能)

Copy link
Member

Choose a reason for hiding this comment

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

nice job!
一般來說遇到這種情況就是盡量不要改動到現有資料使用到的代號(像你選擇的做法),如果真的非不得已需要改動的話,通常會把搬動資料的動作寫在 migration script 裡面(參考 Django 文件

Copy link

codecov bot commented Jul 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.03%. Comparing base (72439bd) to head (0e6ad43).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1207   +/-   ##
=======================================
  Coverage   74.02%   74.03%           
=======================================
  Files          81       81           
  Lines        3061     3062    +1     
=======================================
+ Hits         2266     2267    +1     
  Misses        795      795           

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

@SivanYeh SivanYeh requested a review from mattwang44 July 29, 2024 11:39
Copy link
Member

Choose a reason for hiding this comment

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

這次改動邏輯並不複雜,希望能留單一一份 migration script 就好(可以刪掉兩份後重跑 makemigrations)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed!

feat(sponsor): add diamond level
fix(sponsor): adjust level for backward compatibility
@SivanYeh SivanYeh merged commit 89bbc25 into master Aug 5, 2024
3 checks passed
@SivanYeh SivanYeh deleted the feat/sponsor-diamond branch August 5, 2024 13:38
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