modify some graphql types to Enum#39
Conversation
| const { denominators } = require('../helpers/constants'); | ||
|
|
||
| const typeDef = gql` | ||
| enum ProposalPrlEnum { |
There was a problem hiding this comment.
Add comments or descriptions for each enum.
enum ProposalPrlEnum {
# This is a description
STOPPED
}
helpers/contracts.js
Outdated
|
|
||
| for (const k in contracts) { | ||
| const contract = contracts[k]; | ||
| console.log('constract name = ', contract.contractName); |
There was a problem hiding this comment.
Is this console optional
| NEW: 'ACTIVE', | ||
| }; | ||
|
|
||
| const proposalStages = { |
There was a problem hiding this comment.
Would have preferred normal cased keys like so
const proposalStages = {
idea: 'IDEA',
draft: 'DRAFT',
proposal: 'PROPOSAL',
ongoing: 'ONGOING',
review: 'REVIEW',
archived: 'ARCHIVED',
};
This is fine but CONSTANT_NAMES like these are a fragment of global constant naming so it has perhaps fallen out of favor. Its okay nonetheless.
| # Get actionable statuses for proposals with proposalIds | ||
| getActionableStatus(proposalIds: [String!]): [ProposalActionableObject] | ||
| # Find proposals in specific stage | ||
| fetchProposals(stage: String!, onlyActionable: Boolean): [Proposal] |
There was a problem hiding this comment.
Is stage required? stage: String instead?
There was a problem hiding this comment.
Also, can you use ProposalStageEnum instead of just String?
graphql.js
Outdated
| const { stage, onlyActionable } = args; | ||
| const filter = (stage === 'all') ? {} : { stage: stage.toUpperCase() }; | ||
| const proposals = await getProposals(filter); | ||
| let specialProposals = []; |
There was a problem hiding this comment.
How about usingconst and a ternary to avoid let declaration?
types/proposal.js
Outdated
|
|
||
| # Any actionable status | ||
| actionableStatus: ProposalActionableStatusEnum | ||
| actionableStatus: String |
There was a problem hiding this comment.
Why use String over ProposalActionableStatusEnum?
There was a problem hiding this comment.
Should ProposalActionableStatusEnum be nullable? Is this null when there is no current user? Based on the code it is NONE instead?
There was a problem hiding this comment.
https://www.apollographql.com/docs/graphql-tools/scalars#enums
according to this, I would have to use enum fields like "AWAITING_ENDORSEMENT". But I would like to return strings that replace those underscores with spaces.
graphql.js
Outdated
| actionableStatus: status.replace('_', ' '), | ||
| }; | ||
| }); | ||
| if (onlyActionable) { |
There was a problem hiding this comment.
How about
const allProposals = specialProposals.concat(proposals).map(proposal => ({
...proposal,
actionableStatus: getCurrentActionableStatus(proposal, context.currentUser),
}))
return onlyActionable ?
allProposals.filter(proposal => proposal.actionableStatus !== actionableStatus.NONE) :
allProposals
types/proposal.js
Outdated
| const status = getCurrentActionableStatus(proposal, user); | ||
| return status.replace('_', ' '); | ||
| actionableStatus(proposal, _args, context, _info) { | ||
| if (proposal.actionableStatus && proposal.actionableStatus !== undefined) { |
There was a problem hiding this comment.
How about putting the guard condition first to avoid the lets statement?
if (!context.currentUser) {
return actionableStatus.NONE;
}
const status = proposal.actionableStatus || getCurrentActionableStatus(proposal, context.currentUser);
return status.split('_').join(' ');
2 participants
No description provided.