fix: in web implementation session token is never used during the place autocomplete

[mrcsilverfox]

In web implementation session token is never used during the place autocomplete

[AndreiMisiukevich]

Could you please elaborate on it a bit?

I need to understand how this code is supposed to work.
According to code, _lastSessionToken is always null just because it's never assigned.

Shouldn't it be something like

 return force || _lastSessionToken == null ? (_lastSessionToken = AutocompleteSessionToken()) : _lastSessionToken!;

[matanshukry]

@AndreiMisiukevich ther last session token should be set from the response - not from us.

[mrcsilverfox]

@AndreiMisiukevich Before, the _lastSessionToken was always null, now on each request we check if the session token is valid and set a new session token if needed: _lastSessionToken is null on first request.

[AndreiMisiukevich]

Suggested change

	final sessionToken = _getSessionToken(force: newSessionToken == true);
	final sessionToken = _getSessionToken(force: newSessionToken ?? false);

[matanshukry]

@AndreiMisiukevich it has the same meaning, and == true is what we do in the mobile implementation

[AndreiMisiukevich]

@matanshukry Yeah, I got it.
Just think it doesn't look good. But yeah, it's a personal preference, I think. Not a clean code rule :)

[matanshukry]

I also mentioned that in the comments - but the last session token should be set based on the response -which is missing here.

Also see this: #31 fro the corect usage of the tokens, which it doesn't do.

1. If you match the web package to the mobile (missing setting the value from the response) - we'll merge.
2. If you prefer to go ahead and impleemnt the correct behavior based on the issue - we can merge that too.

[matanshukry]

@AndreiMisiukevich it has the same meaning, and == true is what we do in the mobile implementation

[matanshukry]

@AndreiMisiukevich ther last session token should be set from the response - not from us.

[mrcsilverfox]

@AndreiMisiukevich why the install dependencies failed? Which version of flutter is needed?

[matanshukry]

@mrcsilverfox it's because of the flutter version the CI is using; you can ignore that.

[mrcsilverfox]

I also mentioned that in the comments - but the last session token should be set based on the response -which is missing here.

Also see this: #31 fro the corect usage of the tokens, which it doesn't do.

1. If you match the web package to the mobile (missing setting the value from the response) - we'll merge.
2. If you prefer to go ahead and impleemnt the correct behavior based on the issue - we can merge that too.

Do you think that is enough?

final prom = _svcAutoComplete!.getPlacePredictions(
      AutocompletionRequest()
        ..input = query
        ..origin = origin == null ? null : core.LatLng(origin.lat, origin.lng)
        ..types = typeFilterStr == null ? null : [typeFilterStr]
        ..componentRestrictions = (ComponentRestrictions()..country = countries)
        ..bounds = _boundsToWeb(locationBias)
        ..sessionToken = sessionToken
        ..language = _language,
      (results, status) {
        if (status == PlacesServiceStatus.OK ||
            status == PlacesServiceStatus.ZERO_RESULTS ||
            status == PlacesServiceStatus.NOT_FOUND) {
          _lastSessionToken = sessionToken;
        } else {
          log('API_ERROR: $status');
        }
      },
    );

Or we have to use the completer as used in _getDetails method?

Although I didn't understand why the session token should be set based on the response. Why can't the session token be set in the getSessionToken method?

[mrcsilverfox]

@matanshukry Any updates on this PR?