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

fix: EventStore#getMaxRevisionId can return null #4384

Merged
merged 1 commit into from
Aug 1, 2023

Conversation

acr92
Copy link
Contributor

@acr92 acr92 commented Aug 1, 2023

In a new fresh Unleash instance with cache enabled this can cause feature toggles to never get updated.

We saw in our client that the ETag was ETag: "60e35fba:null" Which looked incorrect for us.

I also did manual testing and if the andWhere had a value of largerThan higher than whatever the id was then we would get back { max: null }.

This should fix that issue.

@sonatype-lift
Copy link

sonatype-lift bot commented Aug 1, 2023

Sonatype Lift is retiring

Sonatype Lift will be retiring on Sep 12, 2023, with its analysis stopping on Aug 12, 2023. We understand that this news may come as a disappointment, and Sonatype is committed to helping you transition off it seamlessly. If you’d like to retain your data, please export your issues from the web console.
We are extremely grateful and thank you for your support over the years.

📖 Read about the impacts and timeline

@vercel
Copy link

vercel bot commented Aug 1, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
unleash-monorepo-frontend ⬜️ Ignored (Inspect) Visit Preview Aug 1, 2023 8:35pm

@vercel
Copy link

vercel bot commented Aug 1, 2023

@acr92 is attempting to deploy a commit to the unleash-team Team on Vercel.

A member of the Team first needs to authorize it.

@ivarconr ivarconr enabled auto-merge (squash) August 1, 2023 19:19
@ivarconr ivarconr disabled auto-merge August 1, 2023 19:52
@ivarconr
Copy link
Member

ivarconr commented Aug 1, 2023

A bit surprised the tests are failing.

@SimenB
Copy link
Contributor

SimenB commented Aug 1, 2023

A bit surprised the tests are failing.

async getMaxRevisionId(): Promise<number> {
if (this.revisionId) {
return this.revisionId;
} else {
return this.updateMaxRevisionId();
}
}
hits else now, so maybe whatever sets it to 16 is never called?

The changes insrc/lib/features/feature-toggle/configuration-revision-service.ts aren't needed (even though they're more correct), but maybe it should be set to 0? Or just left alone even though the number type should then be number | undefined

In a new fresh Unleash instance with cache enabled this can cause
feature toggles to never get updated.

We saw in our client that the ETag was ETag: "60e35fba:null"
Which looked incorrect for us.

I also did manual testing and if the andWhere had a value of
largerThan higher than whatever the id was then we would get
back { max: null }.

This should fix that issue.
@acr92 acr92 force-pushed the fix_super_annoying_type_bug branch from 1e854da to bf7b375 Compare August 1, 2023 20:35
@ivarconr ivarconr merged commit 8aec4a0 into Unleash:main Aug 1, 2023
9 of 10 checks passed
ivarconr pushed a commit that referenced this pull request Aug 2, 2023
In a new fresh Unleash instance with cache enabled this can cause
feature toggles to never get updated.

We saw in our client that the ETag was ETag: "60e35fba:null" Which
looked incorrect for us.

I also did manual testing and if the andWhere had a value of largerThan
higher than whatever the id was then we would get back { max: null }.

This should fix that issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants