-
Notifications
You must be signed in to change notification settings - Fork 7
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
OC-1014: New endpoint to fetch user's drafts #778
base: main
Are you sure you want to change the base?
Conversation
|
||
// WHERE clauses related to the requested publication status of publications. | ||
const versionStatusFilter: Prisma.PublicationWhereInput = | ||
// If the requester is not the account owner, get only publications that have a published version. |
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.
quite hard to read this (as evidenced by the comments needed..), could each conditional be easily separated out?
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.
I think this flattens out as:
!account owner
{ versions: { some: { isLatestLiveVersion: true } } }
account owner + initial drafts
{ versions: { every: { currentStatus: 'DRAFT', versionNumber: 1 } } }
account owner + !initial drafts + versionStatusArray
{ versions: { some: { currentStatus: in: versionStatusArray } } }
account owner + !initial drafts + !versionStatusArray
{}
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.
I'm sure they can; something like this?
let versionStatusFilter: Prisma.PublicationWhereInput;
if (!isAccountOwner) {
versionStatusFilter = {
versions: {
some: {
isLatestLiveVersion: true
}
}
};
} else {
if (initialDraftsOnly) {
versionStatusFilter = {
versions: {
every: {
currentStatus: 'DRAFT',
versionNumber: 1
}
}
};
} else if (versionStatusArray) {
versionStatusFilter = {
versions: {
some: {
currentStatus: {
in: versionStatusArray
}
}
}
};
} else {
versionStatusFilter = {};
}
}
initialDraftsOnly | ||
? { versions: { every: { currentStatus: 'DRAFT', versionNumber: 1 } } } | ||
: // If another version status filter has been supplied, get publications with at least one version with a current status matching the given filters. | ||
versionStatusArray | ||
? { | ||
versions: { |
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.
seems unnecessary and confusing to be multi line here now, it is just
{ versions: { some: { currentStatus: in: versionStatusArray } }
}
The purpose of this PR was to do some back end set up for allowing octopus users to choose a draft they have access to for linking. It specified a new endpoint but it makes more sense to add an option to the existing "get user publications" endpoint to filter to initial drafts only.
The item didn't stipulate this, but non-initial drafts (version 2+) don't apply to this use case, hence the filtering by version number when the new parameter is provided.
It also needs to be able to exclude certain publications, as the user will not need to see the draft they are trying to link from in the search results.
There was version filtering in place already which is a bit confusing, but that was to return publications with at least one version matching the filter. This new filter returns publications with only 1 version which is a bit different, so I thought it made sense to be its own parameter.
Acceptance Criteria:
Checklist:
Tests:
API

E2E
