-
Notifications
You must be signed in to change notification settings - Fork 18
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
Hotfix improve qfround history matching Insertion #1799
Conversation
4498aa8
to
cc79c2c
Compare
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.
LGTM ;) , thx @CarlosQ96
WalkthroughThe pull request introduces several new React components and modifies existing functionality within the AdminJS interface. Key changes include the addition of components for filtering, referencing projects and QF rounds, and bulk updating records. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CustomQfRoundMultiUpdateComponent
participant ApiClient
participant Database
User->>CustomQfRoundMultiUpdateComponent: Initiate bulk update
CustomQfRoundMultiUpdateComponent->>ApiClient: Send bulk update request
ApiClient->>Database: Update records
Database-->>ApiClient: Confirm update
ApiClient-->>CustomQfRoundMultiUpdateComponent: Return success message
CustomQfRoundMultiUpdateComponent-->>User: Display success message
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (10)
Additional comments not posted (6)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 6
const CustomQfRoundReferenceShowComponent = props => { | ||
const { record } = props; | ||
const qfRoundId = | ||
record.params.qfRound?.id || record.params.qfRoundId || 'N/A'; | ||
const href = `/admin/resources/QfRound/records/${qfRoundId}/show`; | ||
|
||
return ( | ||
<ValueGroup label="QF Round"> | ||
<Link href={href} style={{ color: 'inherit', textDecoration: 'none' }}> | ||
{qfRoundId} | ||
</Link> | ||
</ValueGroup> | ||
); | ||
}; | ||
|
||
export default CustomQfRoundReferenceShowComponent; |
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.
Review of CustomQfRoundReferenceShowComponent
This component is well-structured and follows React functional component best practices. However, consider using styled components or external CSS for styling to enhance maintainability.
- Pros: Proper use of destructuring and conditional rendering.
- Cons: Inline styling can be less maintainable than using styled components or CSS classes.
const CustomIdFilterComponent = props => { | ||
const { onChange, property, filter } = props; | ||
const handleChange = event => { | ||
onChange(property.path, event.target.value); | ||
}; | ||
|
||
return ( | ||
<FormGroup> | ||
<Label>{property.label}</Label> | ||
<Input | ||
type="text" | ||
onChange={handleChange} | ||
value={filter[property.path] || ''} | ||
placeholder={`Enter ${property.label} ID`} | ||
style={{ | ||
color: 'white', | ||
backgroundColor: 'rgba(255, 255, 255, 0.1)', // Semi-transparent white background | ||
borderColor: 'rgba(255, 255, 255, 0.3)', // Lighter border for contrast | ||
}} | ||
/> | ||
</FormGroup> | ||
); | ||
}; | ||
|
||
export default CustomIdFilterComponent; |
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.
Review of CustomIdFilterComponent
This component is well-structured and follows React functional component best practices. However, consider using styled components or external CSS for styling to enhance maintainability.
- Pros: Proper use of destructuring, dynamic placeholder text, and handling of input changes.
- Cons: Inline styling can be less maintainable than using styled components or CSS classes.
projectId: row?.projectId, | ||
qfRoundId: row?.qfRoundId, |
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.
Enhancements to getQfRoundActualDonationDetails
The addition of projectId
and qfRoundId
to the getQfRoundActualDonationDetails
function is a positive change, enhancing the function's ability to handle more specific data related to projects and QF rounds.
- Pros: Allows for more granular tracking and processing of donation details.
- Cons: Ensure that these new parameters are fully utilized and tested to confirm their integration.
const CustomQfRoundMultiUpdateComponent = props => { | ||
const [records, setRecords] = useState([ | ||
{ | ||
projectId: '', | ||
qfRoundId: '', | ||
matchingFund: '', | ||
matchingFundAmount: '', | ||
matchingFundPriceUsd: '', | ||
matchingFundCurrency: '', | ||
distributedFundTxHash: '', | ||
distributedFundNetwork: '', | ||
distributedFundTxDate: null, | ||
}, | ||
]); | ||
const [message, setMessage] = useState(''); | ||
|
||
const api = new ApiClient(); | ||
|
||
const addRecord = () => { | ||
setRecords([ | ||
...records, | ||
{ | ||
projectId: '', | ||
qfRoundId: '', | ||
matchingFund: '', | ||
matchingFundAmount: '', | ||
matchingFundPriceUsd: '', | ||
matchingFundCurrency: '', | ||
distributedFundTxHash: '', | ||
distributedFundNetwork: '', | ||
distributedFundTxDate: null, | ||
}, | ||
]); | ||
}; | ||
|
||
const updateRecord = (index, field, value) => { | ||
const updatedRecords = [...records]; | ||
updatedRecords[index][field] = value; | ||
setRecords(updatedRecords); | ||
}; | ||
|
||
const removeRecord = index => { | ||
const updatedRecords = records.filter((_, i) => i !== index); | ||
setRecords(updatedRecords); | ||
}; | ||
|
||
const handleSubmit = async event => { | ||
event.preventDefault(); | ||
setMessage(''); | ||
|
||
try { | ||
const response = await api.resourceAction({ | ||
resourceId: 'QfRoundHistory', | ||
actionName: 'bulkUpdateQfRound', | ||
data: { records }, | ||
}); | ||
|
||
if (response.data.notice) { | ||
if (typeof response.data.notice === 'string') { | ||
setMessage(response.data.notice); | ||
} else if (typeof response.data.notice.message === 'string') { | ||
setMessage(response.data.notice.message); | ||
} else { | ||
setMessage('Update successful'); | ||
} | ||
} else { | ||
setMessage('Update successful'); | ||
} | ||
} catch (error) { | ||
setMessage(`Error: ${error.message}`); | ||
} | ||
}; | ||
|
||
return ( | ||
<Box as="form" onSubmit={handleSubmit}> | ||
<Text variant="lg" fontWeight="bold"> | ||
Update Multiple QfRoundHistory Records | ||
</Text> | ||
{records.map((record, index) => ( | ||
<RecordInput | ||
key={index} | ||
index={index} | ||
record={record} | ||
updateRecord={updateRecord} | ||
removeRecord={removeRecord} | ||
/> | ||
))} | ||
<Button onClick={addRecord} mt="default"> | ||
Add Another Record | ||
</Button> | ||
<Button type="submit" mt="xl"> | ||
Update All | ||
</Button> | ||
{message && <Text mt="default">{message}</Text>} | ||
</Box> | ||
); | ||
}; |
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.
Robust implementation of CustomQfRoundMultiUpdateComponent
.
The main component is implemented effectively with clear state management and functional operations for record handling. The use of asynchronous requests in handleSubmit
is appropriate. Consider enhancing error handling by providing more detailed user feedback or integrating more comprehensive logging mechanisms.
'projectId', | ||
'qfRoundId', |
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.
Confirm integration of new parameters and suggest improvements.
The addition of projectId
and qfRoundId
to the addQfRoundDonationsSheetToSpreadsheet
function is well-integrated into the headers and sheet title. However, consider enhancing the error handling and logging mechanisms to provide more detailed information about these new parameters in case of failures.
bulkUpdateQfRound: { | ||
component: adminJs.bundle( | ||
'./components/CustomQfRoundMultiUpdateComponent', | ||
), | ||
handler: async (request, _response, _context) => { | ||
const { records } = request.payload; | ||
const results: string[] = []; | ||
|
||
for (const record of records) { | ||
const { | ||
projectId, | ||
qfRoundId, | ||
matchingFund, | ||
matchingFundAmount, | ||
matchingFundPriceUsd, | ||
matchingFundCurrency, | ||
distributedFundTxHash, | ||
distributedFundNetwork, | ||
distributedFundTxDate, | ||
} = record; | ||
|
||
const existingRecord = await QfRoundHistory.findOne({ | ||
where: { projectId, qfRoundId }, | ||
}); | ||
|
||
if (existingRecord) { | ||
await QfRoundHistory.createQueryBuilder() | ||
.update(QfRoundHistory) | ||
.set({ | ||
matchingFund, | ||
matchingFundAmount, | ||
matchingFundPriceUsd, | ||
matchingFundCurrency, | ||
distributedFundTxHash, | ||
distributedFundNetwork, | ||
distributedFundTxDate: new Date(distributedFundTxDate), | ||
}) | ||
.where('id = :id', { id: existingRecord.id }) | ||
.execute(); | ||
results.push( | ||
`Updated: Project ${projectId}, Round ${qfRoundId}, Matching Fund: ${matchingFund}`, | ||
); | ||
} else { | ||
results.push( | ||
`Project QfRoundHistory Not found for Project ${projectId}, Round ${qfRoundId}.`, | ||
); | ||
} | ||
} | ||
|
||
return { | ||
notice: { | ||
message: `Operations completed:\n${results.join('\n')}`, | ||
type: 'success', | ||
}, | ||
}; | ||
}, |
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.
Approve new feature and suggest improvements.
The addition of the bulkUpdateQfRound
feature with a custom handler function is a significant enhancement. Consider improving error handling and performance aspects of the handler function to ensure robustness and efficiency.
First steps for Giveth/giveth-dapps-v2#4603
Summary by CodeRabbit
New Features
Enhancements
qfRoundHistoryTab
configuration for better visibility and functionality of properties.Bug Fixes