-
Notifications
You must be signed in to change notification settings - Fork 401
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: restrict versioning by days #4547
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Docker builds report
|
Uffizzi Preview |
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 from a code perspective
I guess it's good to have the code there, but in theory we should never need the logic to blank out the link from the audit log, because the audit log visibility is also restricted for non-enterprise plans. |
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.
See feedback in comment above.
@matthewelwell this has been resolved |
@rolodato resolved button colour in dark mode. I think we're good to go on this ? |
# Conflicts: # frontend/common/services/useFeatureVersion.ts
Sorry @rolodato - I missed this one. The BE has now been merged if that's helpful. In order to manually change the limit though you'll need to directly edit the Note that if you're using an organisation on the scale up plan you will also need to set the |
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.
This looks good to me from a versioning perspective.
The only thing I would add is that we should include an upgrade banner on the Audit Log page stating how much Audit Log history the organisation has access to. This is also available in the audit_log_visibility_days
attribute from the subscription metadata endpoint.
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.
Re-tested manually. Looks good to me.
Thanks for submitting a PR! Please check the boxes below:
docs/
if required so people know about the feature!Changes
Adjusts limiting of version history to date rather than number of versions
How did you test this code?
Manually set limitation in code against https://pr-4433-deployment-54847-flagsmith.app.uffizzi.com/
Disabled links in audit log
Disabled versions in history