Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
ARC-814 RepoSyncState table as source of truth #928
ARC-814 RepoSyncState table as source of truth #928
Changes from 8 commits
19015cc
51e5c2f
99e7967
8d69168
b26d52c
747f037
51922a3
cdfca3d
cd0e9b9
0c76126
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
In which case this method will have to use the default parameter?
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.
Subscription.syncStatus
is optional, we needed a way to handle in the off chance that it wasn't set yet, but almost certainly would be by this point as this is for current subscriptions which any new subscriptions is automatically synced and set toPENDING
. https://github.com/atlassian/github-for-jira/blob/main/src/models/subscription.ts#L166There 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.
does this mean
failedConnections
has always been an unresolved promise?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.
No, it used to be a sync function which I changed to async for the
booleanFlag
callThere 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.
I wonder what's the difference between Subscription and SubscriptionClass? And how does this type change affect things?
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.
yeah, we need to fix that at some point. It's because we're using an export default class called
Subscription
atsrc/models/subscription
but we also have a export const value calledSubscription
which is an instance of the class that's being built by Sequelize and provided bysrc/models/index
. Values and Class types are different and you cannot place a value as a type of a function. If you look at the imports at the top of the file, I had to import both the class type and the value as you need to refer to the value if you want to do operations on the DB with sequelize or else it'll just error at runtime.