add import experiment modal and functionality#2813
add import experiment modal and functionality#2813bcb37 merged 6 commits intoexperiment-design-refreshfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds import experiment modal functionality to the UpGrade platform, enabling users to import experiment designs from JSON files with validation and compatibility checking.
Key Changes
- Added import experiment modal and adapter implementation following the existing pattern for feature flags and segments
- Enhanced error handling to support case-insensitive warning detection and display specific backend error messages
- Connected the import functionality by uncommenting the previously disabled import action
- Added compatibility type information to backend validation responses for better user feedback
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
frontend/projects/upgrade/src/assets/i18n/en.json |
Added translation key for import experiment message |
frontend/projects/upgrade/src/app/shared/services/common-dialog.service.ts |
Added openImportExperimentModal() method to create and configure the import modal with appropriate parameters |
frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-import-modal/common-import-type-adapters.ts |
Implemented ExperimentImportAdapter class to handle experiment validation and import operations |
frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-import-modal/common-import-modal.component.ts |
Updated to use case-insensitive comparison for warning detection in import results |
frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-import-modal/common-import-modal.component.html |
Enhanced to display specific backend error messages with fallback to translation keys |
frontend/projects/upgrade/src/app/features/dashboard/experiments/pages/experiment-root-page/experiment-root-page-content/experiment-root-section-card/experiment-root-section-card.component.ts |
Enabled import functionality by uncommenting the import modal call |
frontend/projects/upgrade/src/app/core/local-storage/local-storage.service.ts |
Added isLoadingImportExperiment state to experiment state initialization |
frontend/projects/upgrade/src/app/core/local-storage/local-storage.service.spec.ts |
Updated test expectations to include the new isLoadingImportExperiment state field |
frontend/projects/upgrade/src/app/core/experiments/store/experiments.selectors.ts |
Added selector for isLoadingImportExperiment state |
frontend/projects/upgrade/src/app/core/experiments/store/experiments.reducer.ts |
Added reducer handler for loading state during experiment import |
frontend/projects/upgrade/src/app/core/experiments/store/experiments.model.ts |
Extended ExperimentState interface with isLoadingImportExperiment property |
frontend/projects/upgrade/src/app/core/experiments/experiments.service.ts |
Added observable and setter method for import loading state |
backend/packages/Upgrade/src/api/services/ExperimentService.ts |
Updated error message prefix and added compatibilityType to all validation response objects |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
@danoswaltCL It is a pre-existing behavior. Should we revisit the spec for this issue? |
| export class ExperimentImportAdapter implements ImportServiceAdapter { | ||
| constructor(private experimentDataService: ExperimentDataService, private experimentService: ExperimentService) {} | ||
|
|
||
| validateFiles(files: any[]): Observable<ValidatedImportResponse[]> { |
There was a problem hiding this comment.
we do have some types defined for files, FeatureFlagFile and SegmentFile, this could be typed the same way probably instead of any.
| <div *ngIf="element.compatibilityType === 'incompatible'"> | ||
| <span *ngIf="config.params.incompatibleMessageKey"> | ||
| {{ config.params.incompatibleMessageKey | translate }} | ||
| {{ element.error || (config.params.incompatibleMessageKey | translate) }} |
There was a problem hiding this comment.
i'm happy we're putting this in here, I couldn't remember what we decided on printing the real error but I think it's good to do even if the error is technical gibberish. @zackcl Do we have design specs for longer text error message like this? (I think this example message where I imported some random .json file looks decent as-is).
There was a problem hiding this comment.
Yeah, I think this looks fine for now. I don’t think we ever defined strict design specs for long error text.
Only thing I noticed is the arrow icon should be down when expanded.
There was a problem hiding this comment.
ok, that's also a pre-existing issue (it happens in prod, somehow we've never notices), i'll make a bug ticket and we can move on, I think everything else in this PR was addressed so I'm approving.
There was a problem hiding this comment.
@zackcl the arrow icon doesn't work for segments or feature flag imports currently. I can fix it in this pr.
i created a separate bug issue |


No description provided.