-
Notifications
You must be signed in to change notification settings - Fork 2
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
Display alert when an old, trial-only binding is added or backfilled #1440
base: main
Are you sure you want to change the base?
Display alert when an old, trial-only binding is added or backfilled #1440
Conversation
? difference( | ||
draftedBindings.map((binding) => | ||
getCollectionName(binding) | ||
), | ||
liveBindings.map((binding) => getCollectionName(binding)) | ||
) | ||
: [], |
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.
We don't need to change anything here... but we could maybe make this a bit eaiser on processing since if we are here we have already done through at least one of these arrays. However, it could end up getting more complex to "cache" off one of these lists while supporting multiple cases.
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.
Actually I guess we always know we have a drafterBindings
here... so potentially worth it to make a shared array that is built up in the loop above. But not sure how that would impact performance with the set
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
@@ -112,6 +125,14 @@ function useUpdateBackfillCounter() { | |||
backfilled && increment === 'false'; | |||
|
|||
if (shouldIncrement || shouldDecrement) { | |||
increment === 'true' |
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.
Since we need to handle this logic in two places... I think we could just store off which array to populate by storing the key up above before we start looping.
Issues
The issues directly below are completely resolved by this PR:
#1409
Changes
1409
The following features are included in this PR:
Extend binding store state to include a top-level, collection-specific metadata property (i.e.,
collectionMetadata
). The property is a dictionary type that is keyed by the catalog name of the collection. The following pieces of metadata are currently stored there:added
,sourceBackfillRecommended
,trialOnlyStorage
, andupdatedAt
.Create a hook,
useTrialPrefixes
, to evaluate, store, and return prefixes that only have the trial storage mapping defined.Create a hook,
useTrialCollections
, to evaluate and return metadata for collections under trial prefixes.Display a warning alert in the binding-level Backfill section when the selected binding is over 20 days old, the corresponding collection is under a trial prefix, and it is backfilled in the current workflow.
Display a warning alert in the top-level Backfill section when the at least one backfilled binding is over 20 days old and the corresponding collection is under a trial prefix.
Display a warning alert in the right pane of the binding selector when the selected binding is over 20 days old, the corresponding collection is under a trial prefix, and it was added in the current workflow.
Store the numeric value of the trial duration of 20 days in an environment variable.
Create component for the alert indicator displayed in the
CollectionConfig
header.Tests
Manually tested
Approaches to testing are as follows:
Validate that the indicator on the Source Collections section header displays in a warning state at least one binding is over 20 days old, the corresponding collection is under a trial prefix, and backfilled or added in the current workflow.
Validate that the indicator on a binding list item displays in a warning state when the binding is over 20 days old, the corresponding collection is under a trial prefix, and backfilled or added in the current workflow.
Validate that the error state of the indicator on a binding list item and the Source Collections header takes precedence over the warning state.
Validate that a warning alert displays in the top-level Backfill section when at least one binding is over 20 days old, the corresponding collection is under a trial prefix, and backfilled in the current workflow.
Validate that a warning alert displays in the binding-level Backfill section when the binding is over 20 days old, the corresponding collection is under a trial prefix, and backfilled in the current workflow.
Validate that a warning alert displays above the Resource Config header when the binding is over 20 days old, the corresponding collection is under a trial prefix, and added in the current workflow.
Validate that source backfill recommended alerts are preserved when the Next button is clicked and the browser refreshed.
Automated tests
N/A
Playwright tests ran locally
Screenshots
Source Collections section header when section contains a source backfill recommended alert
Binding list item when a backfill is recommended for its source
Top-level alert | Backfill
Binding-level alert | Backfill
Binding-level alert | Added