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

presets=https://path/to/presets.json broken #6664

Closed
tordans opened this issue Jul 20, 2019 · 2 comments
Closed

presets=https://path/to/presets.json broken #6664

tordans opened this issue Jul 20, 2019 · 2 comments
Labels
api An issue with the iD API / URL parameters
Milestone

Comments

@tordans
Copy link
Collaborator

tordans commented Jul 20, 2019

It looks to me, that the feature presets=https://path/to/presets.json by @maxgrossman is broken.

This, or I just don't understand how to use it :-).

What I found out so far:

  • In the current master, the .fromExternal method gets a external variable that IMO is not existent. There is also a 404-error that points to this, once I try to load an external preset. This commit fixes the 404, but that is not enough tordans@6a0b0d0

  • The "d3_json -> then -> catch -> finally"-chain at

    iD/modules/presets/index.js

    Lines 286 to 295 in eb0647a

    .then(function(externalPresets) {
    all.build(data.presets, false); // make default presets hidden to begin
    all.build(externalPresets, true); // make the external visible
    })
    .catch(function() {
    all.init();
    })
    .finally(function() {
    done(all);
    });
    seems to be broken. The "then" part is not executed in my tests; its always the catch- + finally-part.

    • Note, initially this used a different try-catch-pattern

      iD/modules/presets/index.js

      Lines 218 to 225 in d87b182

      d3_json(external, function(err, externalPresets) {
      if (err) {
      all.init();
      } else {
      all.build(data.presets, false); // make default presets hidden to begin
      all.build(externalPresets, true); // make the external visible
      }
      done(all);

I also wonder about the correct format, of an "external_presets.json". In my tests I tried using https://github.com/tordans/iD/blob/master/bicycle_parking_custom_preset.json.

  • Is this the right format?
  • What would be a good place to document an example file/file-content?
@maxgrossman
Copy link
Collaborator

@tordans - thanks for the diligence of testing out the feature I wrote 😄

The format of the preset JSON is intended to match the large hashmap that iD reads, seen here.
This format of hashmap is what iD ingests in the non-from-external scenario. When I added this feature, the intent was to just match that.

That said, looking at the #6553 feature since it seems like the preset above is in the iD source tree, using the comma-delimited list of preset names looks like it should work.

As for your second bullet point question, this feature comes in tandem with working my work on maprules. So I do not have a document on how to build this json, but am happy to help you get that tool set up w/iD so you can test out what you intended to.

@bhousel
Copy link
Member

bhousel commented Feb 6, 2020

I've change the code flow in presets/index.js a bit as a result of working on #4994. Now all the presets are essentially external, so we are always deferring the build step. I've renamed build to merge and I've made this function the sort of thing that we can run several times if we want to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api An issue with the iD API / URL parameters
Projects
None yet
Development

No branches or pull requests

4 participants