added new contract task type in the admin dashboard #843
Conversation
|
@PoulavBhowmick03 is attempting to deploy a commit to the LFG Labs Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes introduce a new task type, "Contract," to the admin dashboard, enhancing the existing functionality for managing tasks. This includes updates to type definitions, the addition of a new component for rendering contract details, and modifications to existing components to accommodate the new task type. The implementation allows for the creation of contract-related tasks, aligning with the proposed objectives in the linked issue. Changes
Assessment against linked issues
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (7)
constants/admin.ts (1)
134-135: LGTM with a minor suggestionThe update to the getDefaultValues function correctly handles the new "Contract" task type. This change is consistent with the existing structure and fulfills the PR objective.
Consider adding a TypeScript return type annotation to the getDefaultValues function for improved type safety:
export const getDefaultValues = (type: TaskType): Record<string, unknown> => { // ... existing code ... };This change would make the function's return type explicit and consistent across all cases.
types/frontTypes.d.ts (1)
Line range hint
319-337: Summary: Type definitions for the new "Contract" task type are well-integrated.The changes to
types/frontTypes.d.tsprovide a solid foundation for implementing the new "contract" task type in the admin dashboard. The additions are consistent with the existing type structure and align well with the PR objectives.Next steps:
- Implement the
ContractInputobject if not already done.- Update relevant components and functions to handle the new "Contract" task type.
- Ensure that the admin dashboard UI is updated to include the option for creating "Contract" tasks.
- Add unit tests to cover the new type and its usage.
types/backTypes.d.ts (2)
405-412: LGTM! Consider refining thecallsproperty type.The
CreateContracttype aligns well with the PR objectives and is consistent with other "Create" types in the file. Good job on replacing 'contracts' with 'calls' as per the linked issue.Consider defining a more specific type for the
callsproperty instead of using the broadobjecttype. This could improve type safety and provide better documentation. For example:calls: { [key: string]: { inputs: Array<{ name: string; type: string }>; outputs: Array<{ name: string; type: string }>; }; }This structure allows for multiple function calls, each with its inputs and outputs defined.
414-421: LGTM! Consider refining thecallsproperty type.The
UpdateContracttype is well-structured and consistent with other "Update" types in the file. Making all properties exceptidoptional is a good practice for update operations.As with the
CreateContracttype, consider defining a more specific type for thecallsproperty:calls?: { [key: string]: { inputs: Array<{ name: string; type: string }>; outputs: Array<{ name: string; type: string }>; }; }This will provide better type safety and documentation while maintaining consistency with the
CreateContracttype.components/admin/formSteps/TaskDetailsForm.tsx (1)
108-115: LGTM: ContractStep rendering logicThe conditional rendering for the
ContractStepcomponent is correctly implemented and consistent with other task types. This addition successfully fulfills the PR objective of adding a new contract task type to the admin dashboard.For improved maintainability, consider refactoring the task type checks into a separate function or using a switch statement. This would make it easier to add new task types in the future and improve code readability. For example:
const renderTaskStep = (step: StepMap, index: number) => { switch (step?.type) { case "Quiz": return <QuizStep {...props} />; case "Discord": return <DiscordStep {...props} />; // ... other cases ... case "Contract": return <ContractStep {...props} />; default: return null; } };Then you can simply call
renderTaskStep(step, currentTask)in therenderTaskfunction.app/admin/quests/dashboard/[questId]/page.tsx (2)
49-49: LGTM! Consider a minor improvement for consistency.The addition of the "Contract" type to the
StepMapunion is correct and aligns with the PR objectives. Well done!For consistency with other task types, consider renaming the
ContractInputTypeto something likeContractTaskInputType. This would make it clearer that it's specifically for contract tasks and maintain naming consistency with other input types.
Line range hint
1-725: Consider refactoring for improved code organizationWhile not directly related to the current changes, the file's size and complexity suggest that it might benefit from some refactoring.
Consider breaking down this large component into smaller, more focused components. This could improve readability, maintainability, and testability. Some suggestions:
- Extract the form rendering logic (e.g.,
renderFormStep) into separate components.- Move the task handling functions (
handleAddTasks,handleDeleteTasks,handleUpdateTasks) into a custom hook or utility file.- Consider using the React Context API for managing some of the shared state, which could help reduce prop drilling.
These changes would make the code more modular and easier to understand and maintain in the long run.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
- app/admin/quests/dashboard/[questId]/page.tsx (1 hunks)
- components/admin/formSteps/TaskDetailsForm.tsx (2 hunks)
- components/admin/taskSteps/contractStep.tsx (1 hunks)
- constants/admin.ts (2 hunks)
- types/backTypes.d.ts (1 hunks)
- types/frontTypes.d.ts (3 hunks)
🔇 Additional comments (13)
components/admin/taskSteps/contractStep.tsx (4)
13-17: Component definition looks goodThe
ContractStepcomponent is well-defined as a functional component with proper type annotations. The props are correctly destructured, making the code clean and easy to read.
18-48: Component structure and layout look goodThe component's structure is well-organized, using a flex layout for proper spacing. Each
TextInputis correctly configured with the necessary props, providing a clean and user-friendly interface for entering contract details.Also applies to: 57-58
61-61: Component export is correctThe
ContractStepcomponent is properly exported as the default export, which is the recommended practice for exporting a single component from a file.
1-61: Overall implementation aligns well with PR objectivesThe
ContractStepcomponent successfully introduces the new 'contract' task type in the admin dashboard as required. It reuses fields from existing task types and adds the new 'calls' field for JSON input. The implementation is clean, follows React best practices, and focuses on the front-end creation process as specified in the PR objectives.A few points to consider:
- Add the missing import for the
StepMaptype.- Implement JSON validation for the 'calls' input to fully meet the requirements.
Once these minor adjustments are made, the component will fully satisfy the PR objectives and provide a solid foundation for the new contract task type in the admin dashboard.
constants/admin.ts (2)
21-21: LGTM: New task type added successfullyThe addition of "Contract" to the TASK_OPTIONS array aligns with the PR objective of introducing a new task type in the admin dashboard.
21-21: Summary: New Contract task type successfully implementedThe changes in this file successfully introduce the new "Contract" task type to the admin dashboard, aligning with the PR objectives. The implementation is consistent with the existing code structure and follows the patterns established for other task types.
Key points:
- "Contract" option added to TASK_OPTIONS.
- New ContractInput constant defined with appropriate properties.
- getDefaultValues function updated to handle the new task type.
While the implementation is functional, consider the suggested improvements to enhance type safety and documentation, particularly for the ContractInput constant and the getDefaultValues function.
Overall, good job on implementing the new feature!
Also applies to: 119-125, 134-135
types/frontTypes.d.ts (3)
328-328: LGTM! Type alias is consistent with other input types.The addition of the
ContractInputTypetype alias is consistent with the pattern used for other input types and aligns with the changes made to theStepMaptype.
337-337: LGTM! Verify usage of the new task type.The addition of
"Contract"to theTaskTypeunion completes the set of changes needed to fully integrate the new "contract" task type into the type system.To ensure proper integration, please verify the usage of the new "Contract" task type throughout the codebase. Run the following script:
#!/bin/bash # Description: Verify the usage of the new "Contract" task type # Test: Search for usage of the "Contract" task type rg --type typescript '"Contract"'
319-319: LGTM! Verify the existence ofContractInput.The addition of the
Contracttype to theStepMapunion is consistent with the existing structure and aligns with the PR objective of adding a new "contract" task type.Please ensure that the
ContractInputobject is properly defined elsewhere in the codebase. Run the following script to verify its existence:✅ Verification successful
Re-attempting verification of
ContractInputobject existence.The previous script encountered an error due to an unrecognized file type
typescript. Let's search forContractInputwithout specifying the file type to ensure the definition exists.
Verified:
ContractInputobject is defined inconstants/admin.ts.The presence of
ContractInputensures that theContracttype is fully integrated into theStepMapunion as intended.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of ContractInput object # Test: Search for ContractInput definition rg --type typescript 'const ContractInput = 'Length of output: 70
Script:
#!/bin/bash # Description: Verify the existence of ContractInput object without specifying file type # Search for ContractInput definition across all files rg 'const ContractInput = 'Length of output: 79
types/backTypes.d.ts (1)
405-421: Great job implementing the new contract task type!The addition of
CreateContractandUpdateContracttypes aligns perfectly with the PR objectives and the requirements from the linked issue. These new types provide a clear structure for creating and updating contract tasks in the admin dashboard.Key points:
- The types are consistent with existing patterns in the file.
- The
callsfield has been implemented as requested, replacing thecontractsfield.- No existing types were modified, reducing the risk of breaking changes.
These changes lay a solid foundation for implementing the new contract task type in the admin dashboard.
components/admin/formSteps/TaskDetailsForm.tsx (2)
18-18: LGTM: Import statement for ContractStepThe import statement for
ContractStepis correctly added and follows the existing pattern for importing task step components.
Line range hint
1-265: Summary: Successfully implemented Contract task typeThe changes in this file successfully implement the new Contract task type in the admin dashboard, aligning perfectly with the PR objectives and the requirements outlined in the linked issue #836. The implementation follows the existing patterns and structure of the component, ensuring consistency and maintainability.
Key points:
- The
ContractStepcomponent is correctly imported.- The rendering logic for the Contract task type is properly integrated into the existing conditional structure.
- The changes are minimal and focused, without altering existing functionality.
These modifications effectively extend the
TaskDetailsFormcomponent to handle the new Contract task type, fulfilling the goal of adding this feature to the admin dashboard.app/admin/quests/dashboard/[questId]/page.tsx (1)
Line range hint
559-725: Implement Contract task handling in task management functionsThe
handleAddTasks,handleDeleteTasks, andhandleUpdateTasksfunctions need to be updated to include logic for the new "Contract" task type. Currently, these functions handle all other task types but omit the "Contract" type.To ensure consistent behavior and prevent potential errors, please add the necessary logic for Contract tasks in these functions. Here's an example of how you might implement it in the
handleAddTasksfunction:} else if (step.type === "Balance") { await AdminService.createBalance({ quest_id: questId.current, name: step.data.balance_name, desc: step.data.balance_desc, contracts: step.data.balance_contracts, cta: step.data.balance_cta, href: step.data.balance_href, }); +} else if (step.type === "Contract") { + await AdminService.createContract({ + quest_id: questId.current, + name: step.data.contract_name, + desc: step.data.contract_desc, + // Add other necessary fields for Contract tasks + }); }Similar changes should be made to
handleDeleteTasksandhandleUpdateTasksfunctions.To ensure that the AdminService has the necessary methods for Contract tasks, run the following script:
This will help verify if the backend methods for Contract tasks are already in place or if they need to be added.
There was a problem hiding this comment.
Could you just adjust the Calls placeholder (I forgot to add a specification for it in the issue), with:
e.g.: [{ "contract": "0x...", "entry_point": "transfer", "call_data": ["0x..."], "regex": "..." }]
All the code is good! Just a few little things to add:
in services\authService.ts:
add createContract and updateContract just like for the balance quest
in app\admin\quests\dashboard\[questId]\page.tsx:
update handleAddTasks, handleDeleteTasks and handleUpdateTasks by adding the contract task type
After these quick changes, should be good!
Marchand-Nicolas
added
❌ Change request
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (10)
types/backTypes.d.ts (3)
51-51: Approve the addition ofcallsproperty with a suggestion for improvement.The addition of the
callsproperty to theUserTasktype aligns well with the PR objectives for introducing the newcontracttask type. This change allows for storing contract-related data flexibly.Consider using a more specific type for
callsinstead ofobject | nullto improve type safety. For example:calls: Record<string, unknown> | null;This change would still allow for flexible JSON input while providing better type checking.
406-413: Approve the newCreateContracttype with a suggestion for improvement.The new
CreateContracttype is well-structured and aligns with the PR objectives. It includes all necessary properties for creating a contract task, including thecallsproperty for JSON input.For consistency with other types in the file and to improve type safety, consider using a more specific type for the
callsproperty:export type CreateContract = { quest_id: number; name: string; desc: string; href: string; cta: string; calls: Record<string, unknown>; };This change maintains flexibility for JSON input while providing better type checking.
415-422: Approve the newUpdateContracttype with a suggestion for improvement.The new
UpdateContracttype is well-structured and consistent with other update types in the file. Making all properties exceptidoptional is appropriate for an update operation.For consistency with the suggested improvement in
CreateContractand to improve type safety, consider using a more specific type for thecallsproperty:export type UpdateContract = { id: number; name?: string; desc?: string; href?: string; cta?: string; calls?: Record<string, unknown>; };This change maintains flexibility for JSON input while providing better type checking.
services/authService.ts (3)
359-373: LGTM:createContractfunction implemented correctly.The
createContractfunction follows the established pattern for create functions in this file. It correctly uses theCreateContracttype for parameters and sends a POST request to the appropriate endpoint.Consider enhancing the error handling to return the error or a standardized error object, rather than just logging it. This would allow callers to handle errors more effectively.
} catch (err) { console.log("Error creating contract task", err); + return { error: "Failed to create contract task" }; }
375-389: LGTM:updateContractfunction implemented correctly with minor suggestions.The
updateContractfunction follows the established pattern for update functions in this file. It correctly uses theUpdateContracttype for parameters and sends a POST request to the appropriate endpoint.
- Consider enhancing the error handling to return the error or a standardized error object, similar to the suggestion for
createContract.- For consistency with other functions in the file, consider using
errinstead oferrorin the catch block.- } catch (error) { - console.log("Error updating contract task", error); + } catch (err) { + console.log("Error updating contract task", err); + return { error: "Failed to update contract task" }; }
Line range hint
1-573: Overall assessment: New contract task type successfully implemented.The changes to
services/authService.tseffectively introduce the new contract task type. The implementation is consistent with existing patterns in the file, maintaining code structure and conventions. Minor suggestions have been made for error handling and naming consistency to further improve the code quality.To enhance the maintainability of this growing service file:
- Consider grouping related functions (e.g., all contract-related functions) into separate files or modules.
- Implement a common error handling utility to standardize error responses across all service functions.
- Use a configuration file for API endpoints to centralize URL management.
These refactoring suggestions can be addressed in future PRs to improve the overall architecture of the admin service.
app/admin/quests/dashboard/[questId]/page.tsx (4)
219-230: LGTM with a suggestion: Contract task formattingThe addition of the Contract task type in the tasksFormatter function is consistent with the existing pattern and correctly formats the Contract task data.
However, consider parsing the
callsproperty as JSON if it's expected to be a JSON string. This would ensure that the data is properly structured when used elsewhere in the application.Consider modifying the
contract_callsassignment as follows:- contract_calls: task.calls, + contract_calls: typeof task.calls === 'string' ? JSON.parse(task.calls) : task.calls,This change would ensure that the
callsdata is properly parsed if it's stored as a JSON string in the database.
525-533: LGTM with suggestions: Contract task creationThe addition of the Contract task creation in the handleAddTasks function is consistent with the existing pattern and correctly calls the AdminService.createContract method.
However, consider the following improvements:
- Ensure that the
callsproperty is properly formatted as JSON before sending it to the API.- Add error handling specific to the Contract task creation.
Consider modifying the Contract task creation as follows:
} else if (step.type === "Contract") { await AdminService.createContract({ quest_id: questId.current, name: step.data.contract_name, desc: step.data.contract_desc, href: step.data.contract_href, cta: step.data.contract_cta, - calls: step.data.calls, + calls: JSON.stringify(step.data.contract_calls), }); + } catch (error) { + console.error("Error creating Contract task:", error); + showNotification("Failed to create Contract task. Please try again.", "error"); }These changes ensure that the
callsdata is properly stringified before sending to the API and provide specific error handling for Contract task creation.
632-640: LGTM with suggestions: Contract task updateThe addition of the Contract task update in the handleUpdateTasks function is consistent with the existing pattern and correctly calls the AdminService.updateContract method.
However, consider the following improvements:
- Ensure that the
callsproperty is properly formatted as JSON before sending it to the API.- Add error handling specific to the Contract task update.
- Use the
contract_callsproperty name consistently with the previous parts of the code.Consider modifying the Contract task update as follows:
} else if (step.type === "Contract") { - await AdminService.updateContract({ - id: step.data.id, - name: step.data.contract_name, - desc: step.data.contract_desc, - href: step.data.contract_href, - cta: step.data.contract_cta, - calls: step.data.calls, - }); + try { + await AdminService.updateContract({ + id: step.data.id, + name: step.data.contract_name, + desc: step.data.contract_desc, + href: step.data.contract_href, + cta: step.data.contract_cta, + calls: JSON.stringify(step.data.contract_calls), + }); + } catch (error) { + console.error("Error updating Contract task:", error); + showNotification("Failed to update Contract task. Please try again.", "error"); + } }These changes ensure that the
callsdata is properly stringified before sending to the API, provide specific error handling for Contract task updates, and maintain consistency in property naming.
Line range hint
1-840: Overall assessment: Contract task type implementationThe implementation of the new Contract task type in the quest editing functionality is well-integrated and consistent with the existing patterns in the codebase. The changes appropriately extend the type system, task formatting, creation, and update processes to include the new task type.
However, there are a few areas where the implementation could be improved:
- Consistency in property naming: Ensure that
contract_callsis used consistently instead ofcallsin some places.- JSON handling: The
callsproperty should be stringified when sending to the API and parsed when receiving from the API to ensure proper data structure.- Error handling: Add specific error handling for Contract task creation and updates.
Addressing these points will enhance the robustness and maintainability of the code.
Consider creating a utility function for handling the JSON conversion of the
callsproperty. This would centralize the logic and make it easier to maintain consistency throughout the codebase. For example:const handleContractCalls = (calls: any): string => { return typeof calls === 'string' ? calls : JSON.stringify(calls); };This function could then be used in both the creation and update processes for Contract tasks.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- app/admin/quests/dashboard/[questId]/page.tsx (4 hunks)
- components/admin/taskSteps/contractStep.tsx (1 hunks)
- services/authService.ts (3 hunks)
- types/backTypes.d.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/admin/taskSteps/contractStep.tsx
🔇 Additional comments (4)
types/backTypes.d.ts (1)
Line range hint
1-423: Overall implementation aligns well with PR objectives.The changes to
types/backTypes.d.tsprovide a solid foundation for implementing the new contract task type in the admin dashboard. The additions ofCreateContractandUpdateContracttypes, along with the modification of theUserTasktype, align well with the PR objectives and the requirements outlined in the linked issue.A few minor suggestions have been made to improve type safety, particularly regarding the
callsproperty. These suggestions, if implemented, would maintain the flexibility required for JSON input while providing better type checking.To ensure that these type changes are properly utilized throughout the codebase, consider running the following verification script:
This script will help identify areas where the new types and properties are being used, ensuring consistent implementation across the codebase.
services/authService.ts (2)
25-26: LGTM: Import statements updated correctly.The new types
CreateContractandUpdateContracthave been properly added to the import list, maintaining consistency with other task types in the file.
552-553: LGTM:AdminServiceexport updated correctly.The
createContractandupdateContractfunctions have been properly added to theAdminServiceobject, maintaining the existing order and structure of the export.app/admin/quests/dashboard/[questId]/page.tsx (1)
49-49: LGTM: Contract type added to StepMapThe addition of the Contract type to the StepMap union is consistent with the existing pattern and correctly extends the type system to support the new Contract task type.
| desc: step.data.contract_desc, | ||
| href: step.data.contract_href, | ||
| cta: step.data.contract_cta, | ||
| calls: step.data.calls, |
There was a problem hiding this comment.
Here, this needs to be an object, not a string. You can simply use JSON.parse()
| href: step.data.contract_href, | ||
| cta: step.data.contract_cta, | ||
| calls: step.data.calls, | ||
| }); | ||
| } | ||
| } catch (error) { |
There was a problem hiding this comment.
In this catch, you can add
showNotification(`Error adding ${step.type} task: ${error}`, "error");So that if there is any issue while parsing the JSON, the user is instantly informed
| desc: step.data.contract_desc, | ||
| href: step.data.contract_href, | ||
| cta: step.data.contract_cta, | ||
| calls: step.data.calls, |
There was a problem hiding this comment.
Same here for JSON.parse()
| @@ -607,6 +629,15 @@ export default function Page({ params }: QuestIdProps) { | |||
| cta: step.data.balance_cta, | |||
| href: step.data.balance_href, | |||
| }); | |||
| } else if (step.type === "Contract") { | |||
| await AdminService.updateContract({ | |||
There was a problem hiding this comment.
Also you could wrap this call with a try catch and add show
Notification(`Error updating ${step.type} task: ${error}`, "error");in the catch
|
@Marchand-Nicolas ptal |
|
Hello @PoulavBhowmick03 OD hack ends on Sunday (6). Everything okay ? |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (8)
types/backTypes.d.ts (4)
53-53: LGTM! Consider using a more specific type forcalls.The addition of the
callsproperty to theUserTasktype aligns well with the PR objectives. It allows for storing contract-related data flexibly.For improved type safety, consider defining a more specific type for
callsinstead of usingobject. For example:calls: Record<string, unknown> | null;This provides better type inference while still allowing for flexible JSON-like structures.
408-415: LGTM! Consider using a more specific type forcalls.The
CreateContracttype is well-structured and consistent with other "Create" types in the file. It includes all necessary properties for creating a contract task.For improved type safety and consistency with the
UserTasktype, consider using a more specific type for thecallsproperty:calls: Record<string, unknown>;This provides better type inference while still allowing for flexible JSON-like structures.
417-424: LGTM! Consider using a more specific type forcalls.The
UpdateContracttype is well-structured and consistent with other "Update" types in the file. It correctly includes all necessary properties for updating a contract task, with appropriate optionality.For improved type safety and consistency with the
UserTaskandCreateContracttypes, consider using a more specific type for thecallsproperty:calls?: Record<string, unknown>;This provides better type inference while still allowing for flexible JSON-like structures.
53-53: Summary: New contract task type successfully implemented in type definitions.The changes to this file successfully implement the new
contracttask type as outlined in the PR objectives and linked issue #836. The additions and modifications include:
- A new
callsproperty in theUserTasktype.- A new
CreateContracttype for creating contract tasks.- A new
UpdateContracttype for updating contract tasks.These changes are well-structured and consistent with existing patterns in the file. They provide the necessary type definitions for implementing the new task type in the admin dashboard.
To ensure full alignment with the PR objectives:
- Verify that the backend implementation (which is out of scope for this PR) will be able to handle the
callsfield as a JSON input.- Ensure that the frontend implementation uses these new types correctly when creating and updating contract tasks.
- Consider adding unit tests for these new types to verify their correctness and prevent regressions in future updates.
Also applies to: 408-424
services/authService.ts (3)
Line range hint
361-378: LGTM with a minor suggestionThe
createContractfunction is well-implemented and follows the established pattern for create functions in this file. It correctly sends a POST request to the appropriate endpoint and includes proper error handling.However, there's a minor inconsistency in the error logging message:
Consider updating the error message for consistency:
- console.log("Error creating contract task", err); + console.log("Error while creating contract task", err);This change would align the error message with the style used in other functions throughout the file.
🧰 Tools
🪛 Biome
[error] 364-364: expected
:but instead foundcreateCustomApiRemove createCustomApi
(parse)
Line range hint
380-402: Resolve merge conflict inupdateContractfunctionThere's an unresolved merge conflict in the
updateContractfunction. This will cause syntax errors and prevent the code from running correctly.Please resolve the merge conflict by removing the conflict markers and keeping the correct implementation. The function should follow the pattern of other update functions in the file. Here's a suggested resolution:
const updateContract = async (params: UpdateContract) => { try { const response = await fetch(`${baseurl}/admin/tasks/contract/update`, { method: "POST", headers: { "Content-Type": "application/json", Authorization: `Bearer ${localStorage.getItem("token")}`, }, body: JSON.stringify(params), }); return await response.json(); } catch (error) { console.log("Error while updating contract task", error); } };Make sure to test the function after resolving the conflict to ensure it works as expected.
🧰 Tools
🪛 Biome
[error] 378-378: expected
,but instead found;Remove ;
(parse)
[error] 382-383: Expected a property, a shorthand property, a getter, a setter, or a method but instead found '==='.
Expected a property, a shorthand property, a getter, a setter, or a method here.
(parse)
[error] 383-383: Expected an expression but instead found '==='.
Expected an expression here.
(parse)
[error] 383-383: Expected an expression but instead found '='.
Expected an expression here.
(parse)
[error] 382-383: Invalid assignment to
{ ======This expression cannot be assigned to
(parse)
[error] 384-384: expected
,but instead found;Remove ;
(parse)
[error] 385-386: Expected a catch clause but instead found '}'.
Expected a catch clause here.
(parse)
[error] 382-384: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
Unresolved Merge Conflict Detected in
services/authService.tsA merge conflict remains at line 383 in the
updateContractfunction. Please resolve this conflict to ensure the codebase remains stable and the PR can be approved.🔗 Analysis chain
Line range hint
1-570: Summary of changes and action itemsThe changes to
services/authService.tssuccessfully introduce the new contract task type, aligning with the PR objectives. The implementation follows the established patterns for other task types in this file.Key points to address:
- Resolve the merge conflict in the
updateContractfunction.- Consider updating the error logging message in the
createContractfunction for consistency.Once these items are addressed, the changes will be ready for approval. Great work on implementing the new contract task type!
After resolving the merge conflict, please run the following command to ensure there are no remaining conflict markers:
If this command returns any results, it indicates that there are still unresolved merge conflicts that need to be addressed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for remaining merge conflict markers grep -n "<<<<<<< HEAD\|=======\|>>>>>>> " services/authService.tsLength of output: 79
🧰 Tools
🪛 Biome
[error] 378-378: expected
,but instead found;Remove ;
(parse)
[error] 382-383: Expected a property, a shorthand property, a getter, a setter, or a method but instead found '==='.
Expected a property, a shorthand property, a getter, a setter, or a method here.
(parse)
[error] 383-383: Expected an expression but instead found '==='.
Expected an expression here.
(parse)
[error] 383-383: Expected an expression but instead found '='.
Expected an expression here.
(parse)
[error] 382-383: Invalid assignment to
{ ======This expression cannot be assigned to
(parse)
[error] 384-384: expected
,but instead found;Remove ;
(parse)
[error] 385-386: Expected a catch clause but instead found '}'.
Expected a catch clause here.
(parse)
[error] 382-384: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
app/admin/quests/dashboard/[questId]/page.tsx (1)
Line range hint
1-853: Overall assessment: Contract task type implementation is good, with room for improvement.The implementation of the Contract task type is generally well-done and consistent with existing patterns. However, there are a few areas that could be improved:
JSON parsing: Consider adding error handling for JSON parsing in the createContract function, similar to what's been done in the updateContract function.
Consistency: To maintain consistency, you might want to add similar try-catch blocks for other task types in the handleAddTasks and handleUpdateTasks functions.
Error messages: Consider making error messages more specific to each operation (create, update, delete) for better debugging and user feedback.
To improve overall code quality and maintainability, consider the following:
- Extract common task handling logic into separate functions to reduce code duplication.
- Implement a more robust error handling strategy, possibly using a custom error class for different types of errors (e.g., NetworkError, ValidationError).
- Consider using TypeScript's discriminated unions more effectively to ensure type safety when handling different task types.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
- app/admin/quests/dashboard/[questId]/page.tsx (6 hunks)
- components/admin/formSteps/TaskDetailsForm.tsx (2 hunks)
- constants/admin.ts (3 hunks)
- services/authService.ts (5 hunks)
- types/backTypes.d.ts (2 hunks)
- types/frontTypes.d.ts (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- components/admin/formSteps/TaskDetailsForm.tsx
- constants/admin.ts
🧰 Additional context used
🪛 Biome
services/authService.ts
[error] 378-378: expected
,but instead found;Remove ;
(parse)
[error] 382-383: Expected a property, a shorthand property, a getter, a setter, or a method but instead found '==='.
Expected a property, a shorthand property, a getter, a setter, or a method here.
(parse)
[error] 383-383: Expected an expression but instead found '==='.
Expected an expression here.
(parse)
[error] 383-383: Expected an expression but instead found '='.
Expected an expression here.
(parse)
[error] 382-383: Invalid assignment to
{ ======This expression cannot be assigned to
(parse)
🔇 Additional comments (10)
types/frontTypes.d.ts (4)
Line range hint
319-339: Summary: Contract task type successfully added to type definitionsThe changes to add the new
Contracttask type to the type definitions are well-implemented and consistent with the existing structure. All necessary additions have been made toStepMap,ContractInputType, andTaskType.Next steps:
- Run the provided verification scripts to ensure proper definition and usage of the new types.
- Review the implementation of the
ContractInputconst and the components that use the newContracttask type.- Update any relevant documentation or comments to reflect the addition of the new task type.
Great job on expanding the type system to accommodate the new feature!
329-329: LGTM: Addition of ContractInputTypeThe addition of the
ContractInputTypeas a type alias fortypeof ContractInputis consistent with the existing structure and provides the necessary type for theContractdata in theStepMapunion.To ensure the
ContractInputis properly defined, let's verify its definition:#!/bin/bash # Description: Verify the definition of ContractInput const rg --type typescript "const ContractInput = \{" -A 10
339-339: LGTM: Addition of Contract to TaskType unionThe addition of
Contractto theTaskTypeunion is consistent with the existing structure and aligns with the PR objective. This change ensures thatContractcan be used as a valid task type throughout the application.To ensure the new
Contracttask type is properly utilized, let's verify its usage across the codebase:✅ Verification successful
** Addition of
ContracttoTaskTypehas been successfully verified across the codebase.**All usages of the new
Contracttask type are consistent and align with the intended functionality. No issues were found during the verification process.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the new Contract task type rg --type typescript '"Contract"' -C 2Length of output: 90
Script:
#!/bin/bash # Description: Verify the usage of the new Contract task type across all files rg '"Contract"' -C 2Length of output: 3090
319-319: LGTM: Addition of Contract type to StepMapThe addition of the
Contracttype to theStepMapdiscriminated union is consistent with the existing structure and aligns with the PR objective of adding a new "contract" task type.To ensure consistency across the codebase, let's verify that the
ContractInputtype is properly defined:services/authService.ts (2)
25-26: LGTM: Import statement updated correctlyThe import statement has been appropriately updated to include the new types
CreateContractandUpdateContract. This change aligns with the new functions being added to the file.
569-570: LGTM: AdminService object updated correctlyThe
AdminServiceobject has been properly updated to include the newcreateContractandupdateContractfunctions. This change ensures that these new functions are exported and can be used in other parts of the application, consistent with the pattern for other functions in this service.app/admin/quests/dashboard/[questId]/page.tsx (4)
49-49: LGTM: New Contract task type added to StepMap.The addition of the Contract task type to the StepMap union type is correct and consistent with the existing structure.
220-231: LGTM: Contract task type handling added to tasksFormatter.The implementation for handling the Contract task type in the tasksFormatter function is correct and follows the existing pattern for other task types.
562-562: LGTM: Error handling for task creation.The error handling for task creation is consistent with the existing pattern and provides useful feedback to the user.
658-670: Improved error handling for updateContract.The try-catch block for updating the Contract task is a good addition. It provides specific error handling for this task type.
|
@Marchand-Nicolas PTAL |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
app/admin/quests/create/page.tsx (1)
388-395: Provide user feedback on errors during contract task creationIn the catch block, only logging the error to the console may not be sufficient for user experience. Consider displaying a notification to inform the user that an error occurred while creating the contract task.
Apply this diff to display an error notification:
} catch (error) { console.error("Error while creating contract task:", error); + showNotification("Error while creating contract task", "error"); }
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
app/admin/quests/create/page.tsx (4)
Line range hint
288-294: Correct the error message in the 'Custom' task type validationIn the validation for the 'Custom' task type, the error message incorrectly mentions 'Discord'. It should refer to 'Custom' to match the task type and avoid confusion.
Apply this diff to fix the error message:
} else if (step.type === "Custom") { if ( step.data.custom_name?.length === 0 || step.data.custom_desc?.length === 0 || step.data.custom_cta?.length === 0 || step.data.custom_href?.length === 0 || step.data.custom_api?.length === 0 ) { - showNotification("Please fill all fields for Discord", "info"); + showNotification("Please fill all fields for Custom", "info"); return; } await AdminService.createCustom({ quest_id: questId, name: step.data.custom_name, desc: step.data.custom_desc, cta: step.data.custom_cta, href: step.data.custom_href, api: step.data.custom_api, }); }
Line range hint
380-385: Correct the error message in the 'CustomApi' task type error handlingIn the catch block for the 'CustomApi' task type, the error message incorrectly mentions 'balance task'. It should be updated to 'custom API task' to accurately reflect the task type.
Apply this diff to fix the error message:
} else if (step.type === "CustomApi") { try { await AdminService.createCustomApi({ quest_id: questId, name: step.data.api_name, desc: step.data.api_desc, api_url: step.data.api_url, cta: step.data.api_cta, href: step.data.api_href, regex: step.data.api_regex }); } catch (error) { - console.error("Error while creating balance task:", error); + console.error("Error while creating custom API task:", error); } }
Line range hint
314-320: Add error handling for 'Domain' task typeThe 'Domain' task type lacks error handling to manage exceptions that may occur during the creation of the domain task. For consistency and to handle potential errors gracefully, consider adding a try-catch block similar to other task types.
Apply this diff to add error handling:
} else if (step.type === "Domain") { + try { await AdminService.createDomain({ quest_id: questId, name: step.data.domain_name, desc: step.data.domain_desc, }); + } catch (error) { + console.error("Error while creating domain task:", error); + showNotification(`Error adding ${step.type} task: ${error}`, "error"); + } }
Line range hint
317-327: Add validation for required fields in the 'Balance' task typeThe 'Balance' task type block does not validate that all necessary fields are provided before calling
AdminService.createBalance. Missing required fields can lead to unexpected errors. Consider adding a validation check similar to other task types to prompt the user if any fields are missing.Apply this diff to add validation:
} else if (step.type === "Balance") { + if ( + !step.data.balance_name || + !step.data.balance_desc || + !step.data.balance_contracts || + !step.data.balance_cta || + !step.data.balance_href + ) { + showNotification("Please fill all fields for Balance", "info"); + return; + } try { await AdminService.createBalance({ quest_id: questId, name: step.data.balance_name, desc: step.data.balance_desc, contracts: step.data.balance_contracts, cta: step.data.balance_cta, href: step.data.balance_href, }); } catch (error) { console.error("Error while creating balance task:", error); } }
app/admin/quests/create/page.tsx
Outdated
| } else if (step.type === "Contract"){ | ||
| try { | ||
| await AdminService.createContract({ | ||
| quest_id: questId, | ||
| name: step.data.contract_name, | ||
| desc: step.data.contract_desc, | ||
| href: step.data.contract_href, | ||
| cta: step.data.contract_cta, | ||
| calls: step.data.contract_calls, | ||
| }); | ||
| } catch (error) { | ||
| console.error("Error while creating contract task:", error); | ||
| showNotification(`Error adding ${step.type} task: ${error}`, "error"); | ||
| } |
There was a problem hiding this comment.
Add JSON parsing and error handling for 'calls' field in 'Contract' task type
The 'calls' field in the 'Contract' task type should accept JSON input. Before passing step.data.contract_calls to AdminService.createContract, parse it using JSON.parse to ensure it is valid JSON. If parsing fails, display an error message using showNotification, similar to how it's handled in other task types.
Apply this diff to implement JSON parsing and error handling:
} else if (step.type === "Contract"){
+ try {
+ const calls = JSON.parse(step.data.contract_calls);
+ } catch (e) {
+ showNotification("Invalid JSON in 'calls' field for Contract", "error");
+ return;
+ }
try {
await AdminService.createContract({
quest_id: questId,
name: step.data.contract_name,
desc: step.data.contract_desc,
href: step.data.contract_href,
cta: step.data.contract_cta,
- calls: step.data.contract_calls,
+ calls: calls,
});
} catch (error) {
console.error("Error while creating contract task:", error);
showNotification(`Error adding ${step.type} task: ${error}`, "error");
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } else if (step.type === "Contract"){ | |
| try { | |
| await AdminService.createContract({ | |
| quest_id: questId, | |
| name: step.data.contract_name, | |
| desc: step.data.contract_desc, | |
| href: step.data.contract_href, | |
| cta: step.data.contract_cta, | |
| calls: step.data.contract_calls, | |
| }); | |
| } catch (error) { | |
| console.error("Error while creating contract task:", error); | |
| showNotification(`Error adding ${step.type} task: ${error}`, "error"); | |
| } | |
| } else if (step.type === "Contract"){ | |
| try { | |
| const calls = JSON.parse(step.data.contract_calls); | |
| } catch (e) { | |
| showNotification("Invalid JSON in 'calls' field for Contract", "error"); | |
| return; | |
| } | |
| try { | |
| await AdminService.createContract({ | |
| quest_id: questId, | |
| name: step.data.contract_name, | |
| desc: step.data.contract_desc, | |
| href: step.data.contract_href, | |
| cta: step.data.contract_cta, | |
| calls: calls, | |
| }); | |
| } catch (error) { | |
| console.error("Error while creating contract task:", error); | |
| showNotification(`Error adding ${step.type} task: ${error}`, "error"); | |
| } |
services/authService.ts
Outdated
| const updateContract = async (params: UpdateContract) => { | ||
| try { | ||
| const response = await fetch(`${baseurl}/admin/tasks/contract/update`, { | ||
| ======= |
There was a problem hiding this comment.
Unexpected merge markers
types/frontTypes.d.ts
Outdated
| @@ -316,6 +316,7 @@ type StepMap = | |||
| | { type: "None"; data: object } | |||
| | { type: "Domain"; data: DomainInputType } | |||
| | { type: "Balance"; data: BalanceInputType } | |||
| | { type: "Contract"; data: ContractInputType }; | |||
There was a problem hiding this comment.
Remove the ";" at the end of this line
| } else if (step?.type === "Contract") { | ||
| return ( | ||
| <ContractStep |
There was a problem hiding this comment.
Missing closing parenthesis
|
@Marchand-Nicolas PTAL |
| <TextInput | ||
| onChange={(e) => handleTasksInputChange(e, index)} | ||
| value={step.data.contract_calls || ""} | ||
| name="calls" |
There was a problem hiding this comment.
Still not working, because here it should be contract_calls instead of calls
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Fixes #836
Please add the labels corresponding to the type of changes your PR introduces:
Screenshot
Summary by CodeRabbit
New Features
ContractStepcomponent for inputting contract details.Bug Fixes
Documentation