-
-
Notifications
You must be signed in to change notification settings - Fork 10
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
Feature: License status endpoint #264
Conversation
WalkthroughThe update involves enhancing license management and enforcement in a software system. Changes were made to accurately track and control the number of licensed seats, improve the license status reporting, and ensure that vault creation adheres to license limits. These adjustments are aimed at ensuring compliance with licensing terms, enhancing user experience by providing clear feedback on license status, and preventing unauthorized use of software features beyond the licensed capacity. Changes
Assessment against linked issues
Poem
Recent Review DetailsConfiguration used: CodeRabbit UI Files selected for processing (4)
Files skipped from review as they are similar to previous changes (2)
Additional comments not posted (5)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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 Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (6)
- backend/src/main/java/org/cryptomator/hub/api/BillingResource.java (2 hunks)
- backend/src/main/java/org/cryptomator/hub/api/LicenseResource.java (1 hunks)
- backend/src/main/java/org/cryptomator/hub/license/LicenseHolder.java (2 hunks)
- backend/src/test/java/org/cryptomator/hub/api/BillingResourceTest.java (1 hunks)
- frontend/src/common/backend.ts (3 hunks)
- frontend/src/components/VaultList.vue (6 hunks)
Additional comments: 11
backend/src/main/java/org/cryptomator/hub/api/LicenseResource.java (2)
- 25-33: The implementation of the
/license/status
endpoint looks good and follows RESTful principles and Java EE standards.- 36-45: The
LicenseStatusDto
record is well-defined, making good use of Java records and JSON property annotations for clarity and serialization purposes.backend/src/main/java/org/cryptomator/hub/api/BillingResource.java (1)
- 73-73: The update in the
BillingDto
creation method to uselicenseHolder.getSeats()
aligns with the new licensing model and is correctly implemented.backend/src/main/java/org/cryptomator/hub/license/LicenseHolder.java (2)
- 86-86: Including the exception in the log message enhances error visibility and debugging capabilities. Good improvement.
- 179-183: The renaming of
getNoLicenseSeats
toseatsOnNotExisingLicense
improves clarity, and making it visible for testing is a positive step towards ensuring code quality.backend/src/test/java/org/cryptomator/hub/api/BillingResourceTest.java (1)
- 58-58: The adjustment in the test to expect 5 seats by default correctly reflects the changes in the licensing model. Good update to keep the tests aligned with the application logic.
frontend/src/components/VaultList.vue (3)
- 11-11: Changing the condition to display
LicenseAlert
based onisLicenseSuspicious
is a sensible approach to alert users about potential license issues.- 43-43: The updates to the
MenuButton
attributes, including disabling and styling based onisLicenseSuspicious
, are well-implemented to enforce license compliance.- 143-150: The introduction of
licenseStatus
andisLicenseSuspicious
computed properties is a smart way to dynamically adjust the UI based on the license status. This enhances the user experience by providing immediate feedback on license issues.frontend/src/common/backend.ts (2)
- 198-213: The
LicenseStatusDto
class implementation looks good. However, consider the following:
- Ensure that the date comparison in
isExpired()
accounts for potential timezone differences.- Verify that seat counts (
licensedSeats
andusedSeats
) are always integers to avoid floating-point precision issues.
- 398-399: The addition of
LicenseService
to theservices
object is correctly implemented, ensuring seamless integration and accessibility across the application.
@@ -24,6 +24,7 @@ | |||
import java.time.Instant; | |||
import java.util.Optional; | |||
|
|||
//TODO: redirect ot /license path |
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.
The TODO comment about redirecting to the /license
path should be tracked in an issue to ensure it's addressed in the future.
Would you like me to open a GitHub issue for this task?
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.
LGTM 👍 , would only slightly improve the optional expiration date handling.
backend/src/main/java/org/cryptomator/hub/api/LicenseResource.java
Outdated
Show resolved
Hide resolved
Another thing I noticed: in https://github.com/cryptomator/hub/blob/feature/license-status-endpoint/frontend/src/components/LicenseAlert.vue we still use the billing resource, which means that still only admins see this alert, do we want to adjust this in a separate PR or in this one? |
Co-authored-by: Julian Raufelder <[email protected]>
I changed it in fedd8de |
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.
Actionable comments posted: 1
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.
LGTM 👍
Fixes #262.
This PR adds the new REST endpoint
/license
.This endpoint is intended to give information about the license set in the hub instance. Currently, this endpoint does not have any direct actions, but only a subresource
/license/user-info
. This is intentional, because the longterm goal is to move the/billing
endpoint to/license
.The subresource
/license/user-info
is intended for any user to get necessary information about the license like expiration date and licensed seats.Remark: The license endpoint could also be used in the
VaultDetails
vue component, but due to a required refactoring there it is not used.Summary by CodeRabbit
New Features
LicenseResource
class to provide endpoints for retrieving license status information.LicenseStatusDto
andLicenseService
in the frontend to support license status checks and display.LicenseAlert
based on license status.Bug Fixes
Refactor
LicenseHolder
.Style
MenuButton
attributes inVaultList.vue
for better user interaction based on license status.