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

Add options.query to CZML in order to append tokens to URL/URI #5419

Merged
merged 4 commits into from
Jun 6, 2017

Conversation

ottaviohartman
Copy link
Contributor

For #5408

How does it look? The query is appended to sourceUri. In unwrapUriInterval (which is the only function that looks at sourceUri), it is appended onto czmlInterval or czmlInterval.uri.

I've also updated three unit tests which had mismatched parameters to process.

Ottavio

@ottaviohartman
Copy link
Contributor Author

Once somebody reviews this, I'll finish up the KML changes (which are a little more involved).

@hpinkos
Copy link
Contributor

hpinkos commented Jun 5, 2017

Thanks @omh1280! @mramato could you review this when you have a chance?

@@ -217,7 +221,16 @@ define([
function unwrapUriInterval(czmlInterval, sourceUri) {
var result = defaultValue(czmlInterval.uri, czmlInterval);
if (defined(sourceUri)) {
result = getAbsoluteUri(result, getAbsoluteUri(sourceUri));
var uriComponents = sourceUri.split('?');
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we want to get the parameters from the sourceUri. We need to thread the query object through all of the calls so that we can use it here instead.

@@ -1973,6 +1995,7 @@ define([
* @param {String|Object} czml A url or CZML object to be processed.
* @param {Object} [options] An object with the following properties:
* @param {String} [options.sourceUri] Overrides the url to use for resolving relative links.
* @param {Object} [options.query] Key-value pairs which are appended to all URIs in the CZML.
Copy link
Contributor

Choose a reason for hiding this comment

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

CzmlDataSource.prototype.load and CzmlDataSource.prototype.process most likely need similar updates to their documentation.

@ottaviohartman
Copy link
Contributor Author

updated

@mramato
Copy link
Contributor

mramato commented Jun 6, 2017

Looks great @omh1280! At some point in the future we might want to look into cleaning up the parameter craziness going on in CzmlDataSource (but definitely outside the current scope).

I updated CHANGES, please remember to do this for any API changes or significant bug fixes.

Thanks!

@mramato mramato merged commit 2d13940 into CesiumGS:master Jun 6, 2017
@ottaviohartman ottaviohartman deleted the czml_query branch June 6, 2017 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants