Conversation
A branch for us to review the Allocadia custom connector built for OEM efforts
| }, | ||
| ] | ||
| end | ||
| }, |
There was a problem hiding this comment.
Layout/IndentHash: Indent the right brace the same as the start of the line where the left brace is.
| parse_output: 'date_time_conversion', | ||
| type: 'date_time', | ||
| name: 'updatedDate' | ||
| }, |
There was a problem hiding this comment.
Style/TrailingCommaInArrayLiteral: Avoid comma after the last item of an array.
| }, | ||
|
|
||
| budget: { | ||
| fields: lambda do |_connection, _config_fields| |
There was a problem hiding this comment.
Layout/IndentHash: Use 2 spaces for indentation in a hash, relative to the start of the line where the left curly brace is.
Metrics/BlockLength: Block has too many lines. [65/25]
| end | ||
| }, | ||
|
|
||
| budget: { |
There was a problem hiding this comment.
Layout/AlignHash: Align the elements of a hash literal if they span more than one line.
| title: 'Allocadia', | ||
|
|
||
| methods: { | ||
| make_schema_builder_fields_sticky: lambda do |input| |
There was a problem hiding this comment.
This method is for Custom action! I think this was added to the connector that we built for initial demo.
Don't we need Custom action for Allocadia?
| cells&.each do |id,cell| | ||
| col = columns[id] | ||
|
|
||
| unless col == nil || cell['value'] == nil || cell['value'] == "" |
There was a problem hiding this comment.
Consider using - .blank? or .present? instead
| choiceName = col['choices'].select {|choice| choice['id'] == id}[0]['label'] | ||
| msStr = msStr + choiceName + "::" + pcnt + "," | ||
| end | ||
| value = msStr[0...-1] # trims the trailing , from the end |
There was a problem hiding this comment.
Consider using - value.join(',') - so we can avoid trimming the trailing ,
| col = input[:column] | ||
| value = input[:value] | ||
|
|
||
| value = (value == "") ? nil : value # treat empty string the same as nil |
There was a problem hiding this comment.
Consider using - .presence
| unless value == nil | ||
| case col['type'] | ||
| when 'DROPDOWN' | ||
| value = col['choices'].select {|choice| choice['label'] == value}[0]['id'] |
There was a problem hiding this comment.
Consider using - .dig(..) for retrieving values of 2 or more level deep in nested objects
There was a problem hiding this comment.
I tried to replace with col['choices'].select {|choice| choice['label'] == value}.dig(0,:id) but got an internal error when I ran that. Am I missing something?
There was a problem hiding this comment.
Can you try .dig(0, 'id') instead of .dig(0,:id)?
| itemfilter = input['filter'] ? "?$filter=#{input['filter']}" : "" | ||
| #columns = call(:get_line_item_columns_key_id, { budgetId: input['budgetId'] }) | ||
|
|
||
| budget_items = get("/v1/lineitems/#{itemfilter}").after_response do |code, body, headers| |
There was a problem hiding this comment.
Same feedback as line #834!
| location = headers['location'] | ||
| job_id = location&.split('/')[-1] | ||
| job = get("/v1/jobs/lineitems/#{job_id}") | ||
| error(job) |
There was a problem hiding this comment.
This action will result in an error and will not continue unless it's wrapped in a conditional block!
| }, | ||
|
|
||
| pick_lists: { | ||
| foldersbudgets: ->(_connection) { get('/v1/budgets')&.pluck('name', 'id') || [] }, |
There was a problem hiding this comment.
Same feedback as line #834!
|
|
||
| pick_lists: { | ||
| foldersbudgets: ->(_connection) { get('/v1/budgets')&.pluck('name', 'id') || [] }, | ||
| budgets: ->(_connection) { get('/v1/budgets?$filter=folder eq false')&.pluck('name', 'id') || [] }, |
There was a problem hiding this comment.
Same feedback as line #834!
| pick_lists: { | ||
| foldersbudgets: ->(_connection) { get('/v1/budgets')&.pluck('name', 'id') || [] }, | ||
| budgets: ->(_connection) { get('/v1/budgets?$filter=folder eq false')&.pluck('name', 'id') || [] }, | ||
| folders: ->(_connection) { get('/v1/budgets?$filter=folder eq true')&.pluck('name', 'id') || [] }, |
There was a problem hiding this comment.
Same feedback as line #834!
| ["GRID","GRID"], | ||
| ["OTHER","OTHER"], | ||
| ["PO","PO"], | ||
| ["ROLLUP","ROLLUP"] |
There was a problem hiding this comment.
Style/WordArray: Use %w or %W for an array of words.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
Layout/SpaceAfterComma: Space missing after comma.
| ["DETAILS","DETAILS"], | ||
| ["GRID","GRID"], | ||
| ["OTHER","OTHER"], | ||
| ["PO","PO"], |
There was a problem hiding this comment.
Style/WordArray: Use %w or %W for an array of words.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
Layout/SpaceAfterComma: Space missing after comma.
| ["BUDGET","BUDGET"], | ||
| ["DETAILS","DETAILS"], | ||
| ["GRID","GRID"], | ||
| ["OTHER","OTHER"], |
There was a problem hiding this comment.
Style/WordArray: Use %w or %W for an array of words.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
Layout/SpaceAfterComma: Space missing after comma.
| ["ACTUAL","ACTUAL"], | ||
| ["BUDGET","BUDGET"], | ||
| ["DETAILS","DETAILS"], | ||
| ["GRID","GRID"], |
There was a problem hiding this comment.
Style/WordArray: Use %w or %W for an array of words.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
Layout/SpaceAfterComma: Space missing after comma.
| [ | ||
| ["ACTUAL","ACTUAL"], | ||
| ["BUDGET","BUDGET"], | ||
| ["DETAILS","DETAILS"], |
There was a problem hiding this comment.
Style/WordArray: Use %w or %W for an array of words.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
Layout/SpaceAfterComma: Space missing after comma.
|
|
||
| output_fields: lambda do |object_definitions| | ||
| object_definitions['line_item'].only('itemId') | ||
| end, |
There was a problem hiding this comment.
Style/TrailingCommaInHashLiteral: Avoid comma after the last item of a hash.
| }], | ||
|
|
||
| input_fields: lambda do |object_definitions| | ||
| object_definitions['line_item'].only('name','type','parentId','cells').required('name','type') |
There was a problem hiding this comment.
Layout/SpaceAfterComma: Space missing after comma.
Metrics/LineLength: Line is too long. [102/80]
| inject(:merge) | ||
|
|
||
| post("/v1/budgets/#{input.delete('updateBudgetId')}/lineitems", input.compact) | ||
| .after_response do |code, body, headers| |
There was a problem hiding this comment.
Layout/MultilineMethodCallIndentation: Use 2 (not 1) spaces for indenting an expression spanning multiple lines.
| map { |key, value| { columns[key]['id'] => { 'value' => call(:format_cell_to_allocadia, { value: value, column: columns[key] } ) } } }&. | ||
| inject(:merge) | ||
|
|
||
| post("/v1/budgets/#{input.delete('updateBudgetId')}/lineitems", input.compact) |
There was a problem hiding this comment.
Metrics/LineLength: Line is too long. [86/80]
|
|
||
| input['cells'] = input['cells']&. | ||
| compact&. | ||
| map { |key, value| { columns[key]['id'] => { 'value' => call(:format_cell_to_allocadia, { value: value, column: columns[key] } ) } } }&. |
There was a problem hiding this comment.
Metrics/LineLength: Line is too long. [146/80]
Style/BracesAroundHashParameters: Redundant curly braces around a hash parameter.
Layout/SpaceInsideParens: Space inside parentheses detected.
| %w(GRID GRID), | ||
| %w(OTHER OTHER), | ||
| %w(PO PO), | ||
| %w(ROLLUP ROLLUP) |
There was a problem hiding this comment.
Style/PercentLiteralDelimiters: %w-literals should be delimited by [ and ].
| %w(DETAILS DETAILS), | ||
| %w(GRID GRID), | ||
| %w(OTHER OTHER), | ||
| %w(PO PO), |
There was a problem hiding this comment.
Style/PercentLiteralDelimiters: %w-literals should be delimited by [ and ].
| %w(BUDGET BUDGET), | ||
| %w(DETAILS DETAILS), | ||
| %w(GRID GRID), | ||
| %w(OTHER OTHER), |
There was a problem hiding this comment.
Style/PercentLiteralDelimiters: %w-literals should be delimited by [ and ].
| %w(ACTUAL ACTUAL), | ||
| %w(BUDGET BUDGET), | ||
| %w(DETAILS DETAILS), | ||
| %w(GRID GRID), |
There was a problem hiding this comment.
Style/PercentLiteralDelimiters: %w-literals should be delimited by [ and ].
| [ | ||
| %w(ACTUAL ACTUAL), | ||
| %w(BUDGET BUDGET), | ||
| %w(DETAILS DETAILS), |
There was a problem hiding this comment.
Style/PercentLiteralDelimiters: %w-literals should be delimited by [ and ].
|
|
||
| pick_lists: { | ||
| foldersbudgets: ->(_connection) { get('/v1/budgets')&. | ||
| pluck('name', 'id') || [] }, |
There was a problem hiding this comment.
Layout/BlockEndNewline: Expression at 1206, 67 should be on its own line.
| }, | ||
|
|
||
| pick_lists: { | ||
| foldersbudgets: ->(_connection) { get('/v1/budgets')&. |
There was a problem hiding this comment.
Style/Lambda: Use the lambda method for multiline lambdas.
Layout/MultilineBlockLayout: Block body expression is on the same line as the block start.
| inject(:merge) | ||
|
|
||
| post("/v1/budgets/#{input.delete('updateBudgetId')}/lineitems", | ||
| input.compact) |
There was a problem hiding this comment.
Layout/AlignParameters: Align the parameters of a method call if they span more than one line.
| call(:format_cell_to_allocadia, | ||
| value: value, column: columns[key]) | ||
| } | ||
| } |
There was a problem hiding this comment.
Layout/MultilineHashBraceLayout: Closing hash brace must be on the same line as the last hash element when opening brace is on the same line as the first hash element.
| { 'value' => | ||
| call(:format_cell_to_allocadia, | ||
| value: value, column: columns[key]) | ||
| } |
There was a problem hiding this comment.
Layout/MultilineHashBraceLayout: Closing hash brace must be on the same line as the last hash element when opening brace is on the same line as the first hash element.
| end, | ||
|
|
||
| folders: lambda do |_connection| | ||
| get('/v1/budgets?$filter=folder eq true')&.pluck('name', 'id') || [] |
There was a problem hiding this comment.
Layout/IndentationWidth: Use 2 (not 3) spaces for indentation.
| end, | ||
|
|
||
| budgets: lambda do |_connection| | ||
| get('/v1/budgets?$filter=folder eq false')&.pluck('name', 'id') || [] |
There was a problem hiding this comment.
Layout/IndentationWidth: Use 2 (not 3) spaces for indentation.
|
|
||
| pick_lists: { | ||
| foldersbudgets: lambda do |_connection| | ||
| get('/v1/budgets')&.pluck('name', 'id') || [] |
There was a problem hiding this comment.
Layout/IndentationWidth: Use 2 (not 3) spaces for indentation.
| { 'value' => | ||
| call(:format_cell_to_allocadia, | ||
| value: value, column: columns[key]) | ||
| } |
There was a problem hiding this comment.
Layout/MultilineHashBraceLayout: Closing hash brace must be on the same line as the last hash element when opening brace is on the same line as the first hash element.
| { columns[key]['id'] => | ||
| { 'value' => | ||
| call(:format_cell_to_allocadia, | ||
| value: value, column: columns[key]) |
There was a problem hiding this comment.
Layout/AlignParameters: Align the parameters of a method call if they span more than one line.
| columns = call(:get_line_item_columns_key_name, { budgetId: input.delete('updateBudgetId') }) | ||
|
|
||
| # handle case where it could be a cells object coming in as a string | ||
| if (input['cells']&.is_a?(String)) |
There was a problem hiding this comment.
Style/ParenthesesAroundCondition: Don't use parentheses around the condition of an if.
| "</span> in <span class='provider'>Allocadia</span>", | ||
|
|
||
| execute: lambda do |_connection, input| | ||
| columns = call(:get_line_item_columns_key_name, { budgetId: input.delete('updateBudgetId') }) |
There was a problem hiding this comment.
Style/BracesAroundHashParameters: Redundant curly braces around a hash parameter.
Metrics/LineLength: Line is too long. [101/80]
| of: 'object', | ||
| properties: object_definitions['line_item'] | ||
| }] | ||
| end, |
There was a problem hiding this comment.
Style/TrailingCommaInHashLiteral: Avoid comma after the last item of a hash.
| unless item['parentId'] == nil # filter out grand total row | ||
| item['itemId'] = item.delete 'id' | ||
| item.delete '_links' | ||
| # item['cells'] = call(:format_item_cells, { columns: columns, cells: item['cells'] }) |
There was a problem hiding this comment.
Layout/CommentIndentation: Incorrect indentation detected (column 1 instead of 14).
Metrics/LineLength: Line is too long. [99/80]
| all_items = [] | ||
|
|
||
| budget_items.each do |item| | ||
| unless item['parentId'] == nil # filter out grand total row |
There was a problem hiding this comment.
Layout/IndentationWidth: Use 2 (not 4) spaces for indentation.
Style/Next: Use next to skip iteration.
Style/NilComparison: Prefer the use of the nil? predicate.
sharat-developer
left a comment
There was a problem hiding this comment.
V.Important:
Important:
- #72 (comment)
- #72 (comment)
- #72 (comment)
- #72 (comment)
and in pick_lists - #72 (comment)
Apart from this there are some open comments, but these are the significant ones.
I think, it's better to go thru the V.Important and Important comments once!
| columns = call(:get_line_item_columns_raw, { budgetId: item['budgetId'] }) | ||
|
|
||
| if input['primaryExternalColumnName'] | ||
| primaryColId = columns.select {|col| col['name'] == input['primaryExternalColumnName'] }[0]['id'] |
| if (item['cells'][primaryColId]) | ||
| choiceId = item['cells'][primaryColId]['value'] | ||
| choice = choices.select {|ch| ch['id'] == choiceId }[0] | ||
| externalId = choice['externalAssociations'].size > 0 ? choice['externalAssociations'][0]['externalId'] : nil |
| columns = call(:get_line_item_columns_raw, { budgetId: item['budgetId'] }) | ||
|
|
||
| if input['primaryExternalColumnName'] | ||
| primaryColId = columns.select {|col| col['name'] == input['primaryExternalColumnName'] }[0]['id'] |
| primaryColId = columns.select {|col| col['name'] == input['primaryExternalColumnName'] }[0]['id'] | ||
| choices = get("/v1/budgets/#{item['budgetId']}/columns/#{primaryColId}/choices") | ||
| if (item['cells'][primaryColId]) | ||
| choiceId = item['cells'][primaryColId]['value'] |
| if (item['cells'][primaryColId]) | ||
| choiceId = item['cells'][primaryColId]['value'] | ||
| choice = choices.select {|ch| ch['id'] == choiceId }[0] | ||
| externalId = choice['externalAssociations'].size > 0 ? choice['externalAssociations'][0]['externalId'] : nil |
A branch for us to review the Allocadia custom connector built for OEM efforts