Skip to content

have user_progress report whether user can see lockable stages#9739

Merged
Bjvanminnen merged 2 commits into
stagingfrom
hideStagesNonAuthorized2
Jul 29, 2016
Merged

have user_progress report whether user can see lockable stages#9739
Bjvanminnen merged 2 commits into
stagingfrom
hideStagesNonAuthorized2

Conversation

@Bjvanminnen

Copy link
Copy Markdown
Contributor

Different approach to https://github.com/code-dot-org/code-dot-org/pull/9723/files

I like this approach better - it def feels cleaner. Downside is I think there's potential for a stage to asynchronously appear after we make the user_progress call.

One thing I don't love is that summarize_user_progress is really returning more than just user progress at this point. That was already true before my change, but I'm wondering if we should be renaming this set of methods (downside is this would also mean we would prob want to rename our API route).

/**
* Authorizes the user to be able to see lockable stages
*/
export const authorizeLockable = () => ({ type: AUTHORIZE_LOCKABLE });

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This may eventually go in a different reducer just for tracking lockable related info, but this seems like an okay place for it to start.

private def merge_user_summary(user_data, user)
if user
user_data[:disableSocialShare] = true if user.under_13?
user_data[:lockableAuthorized] = user.authorized_teacher? || user.student_of_authorized_teacher?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Worth making user.lockable_authorized? a method? (If this will be re-used anywhere else.)

@joshlory

Copy link
Copy Markdown
Contributor

One tiny nit: might be worth making the naming of lockableAuthorized and authorizeLockable consistent (unless this is intentional to distinguish them).

LGTM!

@Bjvanminnen

Copy link
Copy Markdown
Contributor Author

One is a verb (authorizeLockable) this other is a noun (lockableAuthorized). Do you think it might be better if I change the verb to setLockablAuthorized?

@joshlory

Copy link
Copy Markdown
Contributor

No strong preference. If it's intentional as-is that SGTM!

@Bjvanminnen Bjvanminnen merged commit 5f3e688 into staging Jul 29, 2016
@Bjvanminnen Bjvanminnen deleted the hideStagesNonAuthorized2 branch July 29, 2016 19:56
deploy-code-org added a commit that referenced this pull request Jul 29, 2016
5bf6038 Merge pull request #9750 from code-dot-org/fix-topinstructions-feature-test (Elijah Hamovitz)
5f3e688 Merge pull request #9739 from code-dot-org/hideStagesNonAuthorized2 (Brent Van Minnen)
deploy-code-org added a commit that referenced this pull request Jul 29, 2016
26780fa Merge pull request #9721 from code-dot-org/netsim-teacher-view (Brad Buchanan)
751ca8a Automatically built. (Continuous Integration)
da1d1b8 Merge pull request #9748 from code-dot-org/bump-block-svg-framed (Elijah Hamovitz)
f646303 Fix tests by passing locale into component (Brad Buchanan)
5bf6038 Merge pull request #9750 from code-dot-org/fix-topinstructions-feature-test (Elijah Hamovitz)
4de1a32 Removes last .new-router-log-modal style (Brad Buchanan)
2821f6e Move most styles into JSX (Brad Buchanan)
5f3e688 Merge pull request #9739 from code-dot-org/hideStagesNonAuthorized2 (Brent Van Minnen)
d327da6 move uitest-specific className from custom Component which does not support classnames to standard paragraph tag which does (Elijah Hamovitz)
c53f0ac Use px for filters marginBottom (Brad Buchanan)
31be25a Move sort methods onto the component (Brad Buchanan)
952a54e Indent conditional component (Brad Buchanan)
50cd059 Restore localized log browser header (Brad Buchanan)
e38b4f8 Manually specify blockSvgFramed clippath width (Elijah Hamovitz)
cf3c84e Merge pull request #9742 from code-dot-org/topInstructions-fix-unwanted-dashed-border (Elijah Hamovitz)
06d30ae Merge pull request #9744 from code-dot-org/fix-survey-view (Brendan Reville)
9b03d61 Merge pull request #9745 from code-dot-org/levelbuilder (Josh Lory)
26501e2 Level builder changes (Continuous Integration)
0cbfc1e Content changes (Continuous Integration)
3d4ef73 Merge branch 'staging' into fix-survey-view (Brendan Reville)
169026f Merge pull request #9702 from code-dot-org/touch-tunneling-fix (Caley Brock)
c863139 Merge pull request #9740 from code-dot-org/prevent-auto-starting-animations (Brad Buchanan)
12ee7ac Merge pull request #9729 from code-dot-org/survey-fixes (Brendan Reville)
c3af7a1 Merge branch 'staging' into survey-fixes (Brendan Reville)
98ace8a Teacher dashboard: Fix teacher view default course (Brendan Reville)
564d930 chat bubble tip stroke should default to none rather than white (Elijah Hamovitz)
9fba967 Clean up test syntax from code review (Caley Brock)
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.

2 participants