Conversation
Contributor
WalkthroughThe changes introduce foundational documentation and configuration for the Changes
Recent review detailsConfiguration used: CodeRabbit UI Files ignored due to path filters (2)
Files selected for processing (5)
Files skipped from review due to trivial changes (4)
Additional comments not posted (2)
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
plugins/view-mapped/src/view-mapped.ts (1)
112-119: Improve error message clarity.The error message in the
catchblock could be more descriptive to help users understand the issue and possible next steps. Consider including more context or potential resolutions.- 'An error occured while updating the workbook. See Event Logs.' + 'An error occurred while updating the workbook. Please check the Event Logs for more details and ensure all configurations are correct.'
Comment on lines
+30
to
+121
| listener | ||
| .filter({ job: 'workbook:viewMappedFieldsOnly' }) | ||
| .on('job:ready', async ({ context: { jobId, workbookId } }) => { | ||
| try { | ||
| // First, we acknowledge the job | ||
| await api.jobs.ack(jobId, { | ||
| info: 'Updating the table to only view mapped fields', | ||
| progress: 10, | ||
| }) | ||
|
|
||
| // Retrieving the info on the custom job we created in the listener above, and storing that info in its own "customJobInfo" variable | ||
| const customJobInfo = await api.jobs.get(jobId) | ||
|
|
||
| // From "customJobInfo" variable, retrieving the jobId specifically of the mapping job that completed, and storing it in its own "mappingJobId" variable | ||
| const mappingJobId = customJobInfo.data.input.mappingJobId | ||
|
|
||
| // Obtaining the mapping job's execution plan to later extract "fieldMapping" out of it, which tells us which fields were mapped in the Matching step | ||
| const jobPlan = await api.jobs.getExecutionPlan(mappingJobId) | ||
|
|
||
| // Initializing an empty array to store the keys of the mapped fields | ||
| const mappedFields = [] | ||
|
|
||
| // Iterating through all destination fields that are mapped and extracting their field keys. Then, pushing keys of mapped fields to the "mappedFields" variable | ||
| for (let i = 0; i < jobPlan.data.plan.fieldMapping.length; i++) { | ||
| const destinationFieldKey = | ||
| jobPlan.data.plan.fieldMapping[i].destinationField.key | ||
|
|
||
| mappedFields.push(destinationFieldKey) | ||
| } | ||
| // Making an API call to only get the "data" property out of the response, and saving it as its own "fetchedWorkbook" variable | ||
| // We need to make this API call and cannot just use what's inside of "workbookOne" because we need data in a specific format | ||
| const { data: workbook } = await api.workbooks.get(workbookId) | ||
|
|
||
| // Looping through all sheets of the Workbook One. For all fields that are mapped, updating those fields' metadata to "{mapped: true}" | ||
| workbook.sheets.forEach((sheet) => { | ||
| sheet.config.fields.forEach((field) => { | ||
| if (mappedFields.includes(field.key)) { | ||
| field.metadata = { mapped: true } | ||
| } | ||
| }) | ||
| }) | ||
|
|
||
| // Looping over each sheet in "workbook" and filtering for fields with metadata "mapped: true". Saving mapped fields per each sheet inside of "filteredWorkbookFields" varibable | ||
| const filteredWorkbookFields = workbook.sheets.map((sheet) => { | ||
| const fields = sheet.config.fields.filter( | ||
| (field) => field.metadata && field.metadata.mapped === true | ||
| ) | ||
| return fields.length > 0 ? fields : null | ||
| }) | ||
|
|
||
| // Updating each sheet in a workbook to only contain fields that a user mapped. This ensures that when the table with data loads, only mapped fields will be displayed | ||
| await api.workbooks.update(workbookId, { | ||
| // Keeping other non-sheet elements of the workbook untouched (Workbook name, its Submit action, etc) | ||
| ...workbook, | ||
|
|
||
| // Mapping over each sheet to update each to only contain fields that are inside of "filteredWorkbookFields" variable (that have metadata "{mapped: true})" | ||
| sheets: workbook.sheets.map((sheet, index) => { | ||
| const mappedWorkbookFields = filteredWorkbookFields[index] | ||
|
|
||
| // If there are no mapped fields, returning the original sheet structure | ||
| if (!mappedWorkbookFields) { | ||
| return sheet | ||
| } | ||
|
|
||
| // If there are mapped fields, returning all properties of the original sheet but updating the "fields" property to the mapped fields | ||
| return { | ||
| ...sheet, | ||
| config: { | ||
| ...sheet.config, | ||
| fields: mappedWorkbookFields, | ||
| }, | ||
| } | ||
| }), | ||
| }) | ||
|
|
||
| // Completing the job with an appropriate message to the user | ||
| await api.jobs.complete(jobId, { | ||
| outcome: { | ||
| message: 'Table update complete. Please audit the data', | ||
| acknowledge: true, | ||
| }, | ||
| }) | ||
| } catch (error) { | ||
| // If something goes wrong while executing the custom job, we fail the job with a message on what next steps to take | ||
| await api.jobs.fail(jobId, { | ||
| outcome: { | ||
| message: | ||
| 'An error occured while updating the workbook. See Event Logs.', | ||
| }, | ||
| }) | ||
| } | ||
| }) |
Contributor
There was a problem hiding this comment.
Optimize field mapping extraction.
The current implementation uses a loop to extract mapped fields. Consider using array methods like map and reduce for a more functional approach, which can improve readability and performance.
- for (let i = 0; i < jobPlan.data.plan.fieldMapping.length; i++) {
- const destinationFieldKey =
- jobPlan.data.plan.fieldMapping[i].destinationField.key
- mappedFields.push(destinationFieldKey)
- }
+ const mappedFields = jobPlan.data.plan.fieldMapping.map(
+ (mapping) => mapping.destinationField.key
+ )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.
Suggested change
| listener | |
| .filter({ job: 'workbook:viewMappedFieldsOnly' }) | |
| .on('job:ready', async ({ context: { jobId, workbookId } }) => { | |
| try { | |
| // First, we acknowledge the job | |
| await api.jobs.ack(jobId, { | |
| info: 'Updating the table to only view mapped fields', | |
| progress: 10, | |
| }) | |
| // Retrieving the info on the custom job we created in the listener above, and storing that info in its own "customJobInfo" variable | |
| const customJobInfo = await api.jobs.get(jobId) | |
| // From "customJobInfo" variable, retrieving the jobId specifically of the mapping job that completed, and storing it in its own "mappingJobId" variable | |
| const mappingJobId = customJobInfo.data.input.mappingJobId | |
| // Obtaining the mapping job's execution plan to later extract "fieldMapping" out of it, which tells us which fields were mapped in the Matching step | |
| const jobPlan = await api.jobs.getExecutionPlan(mappingJobId) | |
| // Initializing an empty array to store the keys of the mapped fields | |
| const mappedFields = [] | |
| // Iterating through all destination fields that are mapped and extracting their field keys. Then, pushing keys of mapped fields to the "mappedFields" variable | |
| for (let i = 0; i < jobPlan.data.plan.fieldMapping.length; i++) { | |
| const destinationFieldKey = | |
| jobPlan.data.plan.fieldMapping[i].destinationField.key | |
| mappedFields.push(destinationFieldKey) | |
| } | |
| // Making an API call to only get the "data" property out of the response, and saving it as its own "fetchedWorkbook" variable | |
| // We need to make this API call and cannot just use what's inside of "workbookOne" because we need data in a specific format | |
| const { data: workbook } = await api.workbooks.get(workbookId) | |
| // Looping through all sheets of the Workbook One. For all fields that are mapped, updating those fields' metadata to "{mapped: true}" | |
| workbook.sheets.forEach((sheet) => { | |
| sheet.config.fields.forEach((field) => { | |
| if (mappedFields.includes(field.key)) { | |
| field.metadata = { mapped: true } | |
| } | |
| }) | |
| }) | |
| // Looping over each sheet in "workbook" and filtering for fields with metadata "mapped: true". Saving mapped fields per each sheet inside of "filteredWorkbookFields" varibable | |
| const filteredWorkbookFields = workbook.sheets.map((sheet) => { | |
| const fields = sheet.config.fields.filter( | |
| (field) => field.metadata && field.metadata.mapped === true | |
| ) | |
| return fields.length > 0 ? fields : null | |
| }) | |
| // Updating each sheet in a workbook to only contain fields that a user mapped. This ensures that when the table with data loads, only mapped fields will be displayed | |
| await api.workbooks.update(workbookId, { | |
| // Keeping other non-sheet elements of the workbook untouched (Workbook name, its Submit action, etc) | |
| ...workbook, | |
| // Mapping over each sheet to update each to only contain fields that are inside of "filteredWorkbookFields" variable (that have metadata "{mapped: true})" | |
| sheets: workbook.sheets.map((sheet, index) => { | |
| const mappedWorkbookFields = filteredWorkbookFields[index] | |
| // If there are no mapped fields, returning the original sheet structure | |
| if (!mappedWorkbookFields) { | |
| return sheet | |
| } | |
| // If there are mapped fields, returning all properties of the original sheet but updating the "fields" property to the mapped fields | |
| return { | |
| ...sheet, | |
| config: { | |
| ...sheet.config, | |
| fields: mappedWorkbookFields, | |
| }, | |
| } | |
| }), | |
| }) | |
| // Completing the job with an appropriate message to the user | |
| await api.jobs.complete(jobId, { | |
| outcome: { | |
| message: 'Table update complete. Please audit the data', | |
| acknowledge: true, | |
| }, | |
| }) | |
| } catch (error) { | |
| // If something goes wrong while executing the custom job, we fail the job with a message on what next steps to take | |
| await api.jobs.fail(jobId, { | |
| outcome: { | |
| message: | |
| 'An error occured while updating the workbook. See Event Logs.', | |
| }, | |
| }) | |
| } | |
| }) | |
| listener | |
| .filter({ job: 'workbook:viewMappedFieldsOnly' }) | |
| .on('job:ready', async ({ context: { jobId, workbookId } }) => { | |
| try { | |
| // First, we acknowledge the job | |
| await api.jobs.ack(jobId, { | |
| info: 'Updating the table to only view mapped fields', | |
| progress: 10, | |
| }) | |
| // Retrieving the info on the custom job we created in the listener above, and storing that info in its own "customJobInfo" variable | |
| const customJobInfo = await api.jobs.get(jobId) | |
| // From "customJobInfo" variable, retrieving the jobId specifically of the mapping job that completed, and storing it in its own "mappingJobId" variable | |
| const mappingJobId = customJobInfo.data.input.mappingJobId | |
| // Obtaining the mapping job's execution plan to later extract "fieldMapping" out of it, which tells us which fields were mapped in the Matching step | |
| const jobPlan = await api.jobs.getExecutionPlan(mappingJobId) | |
| // Using map to extract the keys of the mapped fields | |
| const mappedFields = jobPlan.data.plan.fieldMapping.map( | |
| (mapping) => mapping.destinationField.key | |
| ) | |
| // Making an API call to only get the "data" property out of the response, and saving it as its own "fetchedWorkbook" variable | |
| // We need to make this API call and cannot just use what's inside of "workbookOne" because we need data in a specific format | |
| const { data: workbook } = await api.workbooks.get(workbookId) | |
| // Looping through all sheets of the Workbook One. For all fields that are mapped, updating those fields' metadata to "{mapped: true}" | |
| workbook.sheets.forEach((sheet) => { | |
| sheet.config.fields.forEach((field) => { | |
| if (mappedFields.includes(field.key)) { | |
| field.metadata = { mapped: true } | |
| } | |
| }) | |
| }) | |
| // Looping over each sheet in "workbook" and filtering for fields with metadata "mapped: true". Saving mapped fields per each sheet inside of "filteredWorkbookFields" varibable | |
| const filteredWorkbookFields = workbook.sheets.map((sheet) => { | |
| const fields = sheet.config.fields.filter( | |
| (field) => field.metadata && field.metadata.mapped === true | |
| ) | |
| return fields.length > 0 ? fields : null | |
| }) | |
| // Updating each sheet in a workbook to only contain fields that a user mapped. This ensures that when the table with data loads, only mapped fields will be displayed | |
| await api.workbooks.update(workbookId, { | |
| // Keeping other non-sheet elements of the workbook untouched (Workbook name, its Submit action, etc) | |
| ...workbook, | |
| // Mapping over each sheet to update each to only contain fields that are inside of "filteredWorkbookFields" variable (that have metadata "{mapped: true})" | |
| sheets: workbook.sheets.map((sheet, index) => { | |
| const mappedWorkbookFields = filteredWorkbookFields[index] | |
| // If there are no mapped fields, returning the original sheet structure | |
| if (!mappedWorkbookFields) { | |
| return sheet | |
| } | |
| // If there are mapped fields, returning all properties of the original sheet but updating the "fields" property to the mapped fields | |
| return { | |
| ...sheet, | |
| config: { | |
| ...sheet.config, | |
| fields: mappedWorkbookFields, | |
| }, | |
| } | |
| }), | |
| }) | |
| // Completing the job with an appropriate message to the user | |
| await api.jobs.complete(jobId, { | |
| outcome: { | |
| message: 'Table update complete. Please audit the data', | |
| acknowledge: true, | |
| }, | |
| }) | |
| } catch (error) { | |
| // If something goes wrong while executing the custom job, we fail the job with a message on what next steps to take | |
| await api.jobs.fail(jobId, { | |
| outcome: { | |
| message: | |
| 'An error occured while updating the workbook. See Event Logs.', | |
| }, | |
| }) | |
| } | |
| }) |
bangarang
approved these changes
Aug 8, 2024
Merged
This was referenced Mar 4, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
No description provided.