🇫🇷France @B2F

Thanks daveiano for your active involvment in this project ! Totally agree with this update.

I added you as a project mainteners, you can now also administer releases :)

Oh I see, I couldn't reproduce locally because I use a specific langcode such as:

solrSearch(
    index: "my_index"
    query: $searchQueryString
    offset: $offset
    sort: {
      field: "sort_key"
      order: "desc"
    },
    langcode: "fr"

Thanks for the spotting correcting and reporting the issue(s) !

Thanks @daveiano !

Could you please add a fork on gitlab ?

@godotislate, 11.x done in 3422603- and 3422603-fixing-source-id-for-csv 10.2.x (sorry about branch names)

Branch migrate_source_csv-3413894 ready for merge once core is updated (hopefully).

Also should fix not only spaces but also special characters in migrate source CSV.

We should have address this problem to the root, we don't need to remove such important existing functionalities.

I patched lookupSourceId Sql.php method in Core so that source keys spaces (or any other special characters for that matter?) are not removed anymore:

https://www.drupal.org/project/drupal/issues/3422603 🐛 Fixing source IDs with spaces in Sql.php Fixed

There you go, as I noticed the warning also is symptom of not actually doing the rollback (only in migrate_map, the actual content is not rollbacked), I patch Sql.php in core to map the values to correct IDs.

Diff committed on branch 3422603-fixing-source-ids

b2f → created an issue.

It looks like the only problem is a warning on rollbacks in such case where you have spaces in the header.

[warning] Undefined array key "some CSV header key" Sql.php:232

This warning seems totally non blocking (hence does not justify the constraint in ids) and perhaphs should be tackled in Sql.php ?

it would be preferable to have a code solution to allow spaces in CSV column names and simply remove spaces before mapping columns to source IDs

Totally agree ! And I don't even understand why there is such a heavy restriction on IDs titles since they are not even stored afaik ? The cell value is stored, not the header label, this requirement needs clarification IMO.

Apparently this comes from a problem in \Drupal\migrate\Plugin\migrate\id_map\Sql::getSourceIdsHash https://www.drupal.org/project/migrate_source_csv/issues/3379184 📌 Document that source IDs can only contain alphanumeric characters or underscores Fixed ...

To keep things simple, I agree with #50 and #52 that we should have a configuration (checkbox) to disable the default revoking on user updates.

It can be a pain to *asynchronously* having to revoke and update state of a React app on every (minor) user changes !

On the other hand, of course if the user is deleted related tokens should be deleted.

Thanks for listening, because, actually until now I just applied a patch myself to rollback the changes, otherwise it would have needed a huge rewrite.

Still I hope you could give better evidence of it (may be with a more detailed description, with some code Gists i.e.)and thus give a practical contribution and guidance to the Drupal Leaflet module community ...

I already specifically asked you if you needed more informations about my implementation in comment #10, https://www.drupal.org/project/leaflet/issues/3377403#comment-15190179 💬 This reference is wrong inside Drupal.Leaflet.prototype.create_json Fixed

The thing is there is not a single line of custom JS nor the need to inject a Drupal setting in this case, neither the use of Views module. It's just a controller which returns a standard render array, and it works as is with the leafletRenderMap from the leaflet service, I thought it was clear from my code snippet ... What is missing from my example above ?

That doesn't sound great since a Drupal architecture is first resolving around php classes such as controllers, forcing to do those declarations in JS behaviors is adding a significant overhead to go back & forth between JS and php, although before d10 update it was perfectly fine.

Especially if the json data is held into entity fields and multiple maps are managed around a factored map structure.

What is the proper way to address this issue 6 years later in d10 ? Since the latest update is breaking backward compatibility with json layer in our case ...

Doesn't it breaks https://www.drupal.org/project/leaflet/issues/3186029 → ?

Could you please explain where to go from the above because your d10 update breaks the geojson layers in our case.

Can you kindly please be more specific with ?

Whichever is the way you generate the Drupal Leaflet map you could (and should) use that approach to add/inject GeoJson resources into that …

?

I mean can't you see how much we were apparently relying on this feature ?
It's like if I got punished to open this issue because you wouldn't have seen this in the first place ? Come on' ...

Commited to 2.0.x-dev, thanks

Leaflet Map View as in Views module ?

Sorry but no we were not using the Views module, as described in my previous comment I explained "The map is displayed in a typical controller through the method above".

Hello itamair,

I made a wrapper around leafmet.service with the following method:

  /**
   * @param array $geoJsons
   * @param string $mapName
   *   Voir fei_maps_leaflet_map_info().
   * @param string $mapHeight
   *
   * @return array
   *   Render.
   */
  protected function buildGeoJsonMap(array $geoJsons, $mapName = 'carte_pour_geojsons', $mapHeight = '800px') {

    $this->addGeojsonWarnings($geoJsons);

    $map = leaflet_map_get_info($mapName);
    $features = [];
    foreach ($geoJsons as $tid => $geojson) {
      $geoJsonArray = json_decode($geojson, TRUE);
      // Keep the related tid reference for the click event:
      $geoJsonArray['tid'] = $tid;
      $features[] = [
        'type' => 'json',
        'json' => $geoJsonArray,
        // @see https://www.drupal.org/project/leaflet/issues/3186029#comment-13927181
        'events' => [
          'click' => 'Drupal.manageGeojsonClick',
        ],
      ];
    }
    return $this->leaflet->leafletRenderMap($map, $features, $mapHeight);
  }

The $geoJsons array is containing typical geojson strings as such:

"{"type": "Feature", "properties": { "scalerank": 1, "featurecla": "Admin-0 country", "labelrank": 2.0, "sovereignt": "Argentina", "sov_a3": "ARG", "adm0_dif": 0.0, "level": 2.0, "type": "Sovereign country", "admin": "Argentina", "adm0_a3": "ARG", "geou_dif": 0.0, "geounit": "Argentina", "gu_a3": "ARG", "su_dif": 0.0, "subunit": "Argentina", "su_a3": "ARG", "brk_diff": 0.0, "name": "Argentina", "name_long": "Argentina", "brk_a3": "ARG", "brk_name": "Argentina", "brk_group": null, "abbrev": "Arg.", "postal": "AR", "formal_en": "Argentine Republic", "formal_fr": null, "note_adm0": null, "note_brk": null, "name_sort": "Argentina", "name_alt": null, "mapcolor7": 3.0, "mapcolor8": 1.0, "mapcolor9": 3.0, "mapcolor13": 13.0, "pop_est": 40913584.0, "gdp_md_est": 573900.0, "pop_year": -99.0, "lastcensus": 2010.0, "gdp_year": -99.0, "economy": "5. Emerging region: G20", "income_grp": "3. Upper middle income", "wikipedia": -99.0, "fips_10": null, "iso_a2": "AR", "iso_a3": "ARG", "iso_n3": "032", "un_a3": "032", "wb_a2": " [...]

Looks like it was taken straight out of a dataset such as: https://github.com/datasets/geo-boundaries-world-110m/blob/master/countr...

Then the map is displayed in a typical controller through the method above. Not sure if you need more info about the map settings for example ?

I don't think calls to this in Drupal.Leaflet.prototype.create_feature are an issue since they are called at the prototype's function level. Furthermore I would probably have seen the error also there if it was necessary to modify create_feature, my patch alone seems to resolve the issue mentioned in the description.

b2f → created an issue.