Skip to content
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

fix: Dart uuid generation #1071

Merged
merged 5 commits into from
Feb 5, 2022
Merged

fix: Dart uuid generation #1071

merged 5 commits into from
Feb 5, 2022

Conversation

M123-dev
Copy link
Member

@M123-dev M123-dev commented Feb 5, 2022

Now generation a random uuid and storing it instead of grabbing a local identifyer

Copy link
Contributor

@monsieurtanuki monsieurtanuki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job @M123-dev!
Still some minor renaming to do: please have a look at my comments.

import 'package:smooth_app/pages/user_preferences_dev_mode.dart';
import 'package:uuid/uuid.dart';

import 'local_database.dart';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency, please use a 'package:smooth_app/...' path.

@@ -42,12 +45,22 @@ abstract class ProductQuery {

/// Device Id, potentially used as API uuid.
static String? deviceId;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't be afraid to remove deviceId. It's not a device id, and anyway it's the same as OFFConfig.uuid.


/// Sets the device id as "final variable", for instance for API queries.
///
/// To be called at main / init.
static Future<void> setDeviceId() async => OpenFoodAPIConfiguration.uuid =
deviceId = await PlatformDeviceId.getDeviceId;
static Future<void> setDeviceId(final LocalDatabase _localDatabase) async {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rename a setUuid.

@M123-dev
Copy link
Member Author

M123-dev commented Feb 5, 2022

Thanks for your review @monsieurtanuki, applied your comments

static const String UUID_NAME = 'UUID_NAME';

/// Sets the device id as "final variable", for instance for API queries.
///
/// To be called at main / init.
static Future<void> setDeviceId(final LocalDatabase _localDatabase) async {
static Future<void> setUuid(final LocalDatabase _localDatabase) async {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A file is missing, right? The one that called setDeviceId...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just noticed that too

Copy link
Contributor

@monsieurtanuki monsieurtanuki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @M123-dev: I have minor comments that will take you 5 seconds to fix.
Feel free to merge regardless if you are in a hurry.

@@ -40,14 +42,21 @@ abstract class ProductQuery {
'_'
'${getCountry()!.iso2Code.toUpperCase()}';

/// Device Id, potentially used as API uuid.
static String? deviceId;
static const String UUID_NAME = 'UUID_NAME';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be private.

@@ -40,14 +42,21 @@ abstract class ProductQuery {
'_'
'${getCountry()!.iso2Code.toUpperCase()}';

/// Device Id, potentially used as API uuid.
static String? deviceId;
static const String UUID_NAME = 'UUID_NAME';

/// Sets the device id as "final variable", for instance for API queries.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not "device id" but "uuid"

@M123-dev
Copy link
Member Author

M123-dev commented Feb 5, 2022

Applied both

@M123-dev M123-dev merged commit 5c68cfc into develop Feb 5, 2022
@M123-dev M123-dev deleted the uuid-fix branch February 5, 2022 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants