fix(spans-migration): changed type and wording of dropped fields warnings #101479
Conversation
| const combinedWarnings = [ | ||
| ...columnsDropped, | ||
| ...equationsDroppedParsed, | ||
| ...orderbyDroppedWithoutNegation, | ||
| ]; | ||
| const allWarningsSet = new Set(combinedWarnings); | ||
| const allWarnings = [...allWarningsSet]; |
There was a problem hiding this comment.
doing this because some of the fields could be duplicated and we don't need to see duplicate fields
| }) | ||
| orderbyDropped.push( | ||
| ...changedReason.orderby.flatMap(orderby => | ||
| typeof orderby.reason === 'string' ? orderby.orderby : orderby.reason |
There was a problem hiding this comment.
Bug: Orderby Reason Incorrectly Added as Field Name
The flatMap for orderbyDropped sometimes adds orderby.reason (which can be a string array) instead of the orderby.orderby field name. This causes the warning message to incorrectly display reasons as dropped fields, and may lead to a runtime error when string operations are performed on an array.
| typeof orderby.reason === 'string' ? orderby.reason : orderby.reason.join(', '); | ||
|
|
||
| // make sure that the reason and the orderby aren't the same and word it correctly | ||
| return typeof orderby.reason === 'string' || orderbyWithoutPrefix === reasonText |
There was a problem hiding this comment.
Bug: Incorrect Orderby Warning Logic
The DroppedFieldsAlert component's orderby warning logic incorrectly determines the message to display. The condition meant to identify if the orderby field itself is the reason for being dropped misinterprets both string and array reason types, resulting in an inaccurate warning message.
changed the changed reason types on the frontend to reflect the changes i've made to the backend. I've also changed up the wording for the frontend errors as suggested. Widgets have been simplified to one sentence and queries have slightly changed wording to not use "dropped"