feat!: use plugin options for optional plugin parameters #4762
Conversation
NikaHsn
force-pushed
the
feat/plugin_options
branch
4 times, most recently
from
0208571 to
fdd2423
Compare
| /// {@template amplify_datastore.datastore_plugin_options} | ||
| /// The plugin options for the Amplify DataStore plugin. | ||
| /// {@endtemplate} | ||
| class DataStorePluginOptions { |
There was a problem hiding this comment.
I think the plugin option definitions should live in core
There was a problem hiding this comment.
I think we should maintain category-specific types within the core and keep types exclusive to plugins within their respective package.
| @@ -39,8 +39,7 @@ const _v4analytics = ''' | |||
| }, | |||
| "pinpointTargeting": { | |||
| "region": "$REGION" | |||
| }, | |||
| "autoFlushEventsInterval": $ANALYTICS_FLUSH_INTERVAL | |||
| } | |||
There was a problem hiding this comment.
I don't have the full context on these tests, but I believe the intention is to test against the output from various versions of the Amplify CLI. These files are generated using an npm script, which I think depends on different versions of the Amplify CLI. If we regenerate these files these changes will be removed.
The approach I took in the auth PR was to remove certain keys from the JSON in the test assertions. You can see that here: https://github.com/aws-amplify/amplify-flutter/pull/4764/files#diff-10ab98843576a1782a1c3212f13233440a0474bf716ee5f2b70eb2264356d8ebR37
Long term I am not sure how useful these tests are. They are testing against versions of the CLI that are pretty outdated now. I think they should be updated or removed. I think it will be easier to do that as a follow up after we get the breaking changes in though.
There was a problem hiding this comment.
the file is auto generated by running the generate script. CLI does not generate this feild, based on our docs this field must be manually added to the configuration. https://docs.amplify.aws/flutter/build-a-backend/more-features/analytics/record-events/#flush-events
| @@ -54,6 +54,9 @@ class _MyAppState extends State<MyApp> { | |||
| AmplifyAnalyticsPinpoint( | |||
| // ignore: invalid_use_of_visible_for_testing_member | |||
| secureStorageFactory: storageFactory, | |||
| options: const AnalyticsPinpointPluginOptions( | |||
There was a problem hiding this comment.
note: We should call this change out in the migration guide
| /// {@template amplify_datastore.datastore_plugin_options} | ||
| /// The plugin options for the Amplify DataStore plugin. | ||
| /// {@endtemplate} | ||
| class DataStorePluginOptions { |
There was a problem hiding this comment.
suggestion: This should probably be in a separate file
| /// {@template amplify_api_dart.api_plugin_options} | ||
| /// The plugin options for the Amplify API plugin. | ||
| /// {@endtemplate} | ||
| class APIPluginOptions { |
| @@ -21,18 +21,15 @@ import 'package:meta/meta.dart'; | |||
| class AmplifyAPIDart extends APIPluginInterface with AWSDebuggable { | |||
| /// {@macro amplify_api_dart.amplify_api_dart} | |||
| AmplifyAPIDart({ | |||
| List<APIAuthProvider> authProviders = const [], | |||
| APIPluginOptions options = const APIPluginOptions(), | |||
| ConnectivityPlatform connectivity = const ConnectivityPlatform(), | |||
There was a problem hiding this comment.
this is intended for the API flutter package to provide platform-specific implementations for ConnectivityPlatform, and it is not related to plugin options.
NikaHsn
force-pushed
the
feat/plugin_options
branch
from
fdd2423 to
ff38d1d
Compare
| @@ -0,0 +1,40 @@ | |||
| import 'package:amplify_core/amplify_core.dart'; | |||
There was a problem hiding this comment.
| import 'package:amplify_core/amplify_core.dart'; | |
| // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | |
| // SPDX-License-Identifier: Apache-2.0 | |
| import 'package:amplify_core/amplify_core.dart'; |
| @@ -0,0 +1,12 @@ | |||
| /// {@template amplify_analytics_pinpoint_dart.analytics_pinpoint_plugin_options} | |||
There was a problem hiding this comment.
| /// {@template amplify_analytics_pinpoint_dart.analytics_pinpoint_plugin_options} | |
| // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | |
| // SPDX-License-Identifier: Apache-2.0 | |
| /// {@template amplify_analytics_pinpoint_dart.analytics_pinpoint_plugin_options} |
| @@ -0,0 +1,26 @@ | |||
| import 'package:amplify_core/amplify_core.dart'; | |||
There was a problem hiding this comment.
| import 'package:amplify_core/amplify_core.dart'; | |
| // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | |
| // SPDX-License-Identifier: Apache-2.0 | |
| import 'package:amplify_core/amplify_core.dart'; |
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.