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

Bug fixes #8

Open
wants to merge 3 commits into
base: staging
Choose a base branch
from
Open

Bug fixes #8

wants to merge 3 commits into from

Conversation

quantiom
Copy link

@quantiom quantiom commented Aug 4, 2022

No description provided.

Copy link
Owner

@birthdates birthdates left a comment

Choose a reason for hiding this comment

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

What exactly is wrong with this check?

@quantiom
Copy link
Author

quantiom commented Aug 6, 2022

It will never be true. From what it looks like (and from my testing), it checks if getTransactionRedisKey(txn_id) exists in Redis before it ever adds it.

@birthdates
Copy link
Owner

It will never be true. From what it looks like (and from my testing), it checks if getTransactionRedisKey(txn_id) exists in Redis before it ever adds it.

redisClient.set(getRedisKey(session), JSON.stringify(data), "EX", 2592000);

It's created here?

@quantiom
Copy link
Author

quantiom commented Aug 6, 2022

It will never be true. From what it looks like (and from my testing), it checks if getTransactionRedisKey(txn_id) exists in Redis before it ever adds it.

redisClient.set(getRedisKey(session), JSON.stringify(data), "EX", 2592000);

It's created here?

That's storing the result of getRedisKey, which is not the same as the result of getTransactionRedisKey (the one that it checks for)

@birthdates
Copy link
Owner

It will never be true. From what it looks like (and from my testing), it checks if getTransactionRedisKey(txn_id) exists in Redis before it ever adds it.

redisClient.set(getRedisKey(session), JSON.stringify(data), "EX", 2592000);

It's created here?

That's storing the result of getRedisKey, which is not the same as the result of getTransactionRedisKey (the one that it checks for)

Hmmm weird, from my testing it seemed to work. I'll do some testing on my end before I merge this.

@quantiom
Copy link
Author

quantiom commented Aug 9, 2022

Have you tested it?

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