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

expose pairing_channel_id, use with getPathName() for pairing URL #18141

Merged
merged 3 commits into from Oct 5, 2017

Conversation

cpirich
Copy link
Contributor

@cpirich cpirich commented Oct 4, 2017

  • Trying to generate a project view URL from ruby code in the dashboard turns out to be very hard to do. Other similar code relies on a magic client-side function called getStandaloneApp() - see
  • Rather than generate a pairing_attempt URL for channel-backed levels, we instead generate a pairing_channel_id and move the URL generation to the client-side where we already have access to the special client-side code

@@ -509,6 +509,11 @@ StudioApp.prototype.alertIfCompletedWhilePairing = function (config) {
{msg.pairingNavigatorLink()}
</a>
}
{config.level.pairingChannelId &&
<a href={window.dashboard.project.getPathName('view', config.level.pairingChannelId)}>
Copy link
Contributor

Choose a reason for hiding this comment

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

We now import project (rather than relying on window.dashboard), so I think you can just do project.getPathName

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

* @returns {string} Url to the specified action.
* @throws {Error} If this type of project does not have a standalone app.
*/
getPathName(action) {
var pathName = this.appToProjectUrl() + '/' + this.getCurrentId();
getPathName(action, projectId) {
Copy link
Contributor

Choose a reason for hiding this comment

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

with ES6 we can express this as getPathName(action, projectId = this.getCurrentId)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

var pathName = this.appToProjectUrl() + '/' + this.getCurrentId();
getPathName(action, projectId) {
projectId = projectId || this.getCurrentId();
var pathName = this.appToProjectUrl() + '/' + projectId;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: let instead of var

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

@@ -307,7 +307,7 @@ def user_progress_for_stage
elsif level.channel_backed?
@level = level
recent_channel = safe_get_channel_for(level, recent_user) if recent_user
response[:pairingAttempt] = send("#{level.game.app}_project_view_projects_url".to_sym, channel_id: recent_channel) rescue nil if recent_channel
response[:pairingChannelId] = recent_channel if recent_channel
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this approach of sending a channel Id instead 👍

Copy link
Contributor

@Bjvanminnen Bjvanminnen left a comment

Choose a reason for hiding this comment

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

few small nits, but otherwise looks good. i like the new approach :)

@cpirich cpirich merged commit 6477276 into staging Oct 5, 2017
@cpirich cpirich deleted the expose_pairing_channel_id branch October 5, 2017 01:50
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.

None yet

2 participants