-
Notifications
You must be signed in to change notification settings - Fork 18
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: fixed audience counter permission problem #208
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review done.
val userRoles = member?.roles?.map { it.id } ?: emptyList() | ||
val requiredRole = wsa.wsaAlphaRoleId | ||
|
||
reply("counter start!!!").queue { | ||
scheduledRecordHighestAudience(startRecordTime, endRecordTime) | ||
if (userRoles.contains(requiredRole)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- 從
Member
取出的Id
列表應該是memberRoleIds
wsa.wsaAlphaRoleId
已經是具有意義的屬性,可以不用再包成一個變數- 可以再思考這三行程式碼能不能再簡化成更語意化的方法
...nce-counter/src/main/kotlin/tw/waterballsa/utopia/audiencecounter/AudienceCounterListener.kt
Outdated
Show resolved
Hide resolved
...nce-counter/src/main/kotlin/tw/waterballsa/utopia/audiencecounter/AudienceCounterListener.kt
Outdated
Show resolved
Hide resolved
Kudos, SonarCloud Quality Gate passed! |
val currentTime = now() | ||
val startRecordTime = currentTime.truncatedTo(ChronoUnit.MINUTES).toDate() | ||
val endRecordTime = currentTime.plusMinutes(recordPeriodTime).toDate() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: 把 currentTime
、 startRecordTime
、 endRecordTime
封裝成一個具有領域意義類別,程式會更乾淨整潔
請參考 message-cherry-pick
的 DateTimeRange
。
Why need this change? / Root cause:
The counter command should be used by Alpha.
Changes made:
Test Scope / Change impact:
if user is Alpha -> run the counter
if user isn't Alpha -> tell the user "You should be the Alpha."