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

Access report: temporarily hide the banner on the teacher homepage #21644

Merged
merged 1 commit into from
Apr 4, 2018

Conversation

breville
Copy link
Member

@breville breville commented Apr 3, 2018

We discussed programmatically reactivating on a set date, but the date of the reappearance and its coordination with removing other banners is still in flux.

@breville breville requested a review from sureshc April 4, 2018 04:08
def show_census_teacher_banner?
# Temporarily hide the banner
Copy link
Contributor

Choose a reason for hiding this comment

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

How temporary? And will we want to be able to override hide/show like this in the future? If so, we'll probably want a more robust solution than disabling rubcop and returning false. If temporary means a couple days and we don't anticipate needing to do this again, then I guess it seems fine.

Copy link
Member Author

Choose a reason for hiding this comment

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

Great question. It will be hidden for roughly 2.5 weeks, and turning it back on will be done in sync with flipping another banner off which is why we decided to do it via manual PRs for now. I'm not sure whether we have a longer-term schedule since this particular call to action is relative new territory for us. Totally agreed that if we see this occurring too often we should build a better flipping solution.

Copy link
Member Author

Choose a reason for hiding this comment

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

The most obvious immediate alternative would be to simply replace the function with false but I prefer keeping code intact that we know we're about to revive.

@breville
Copy link
Member Author

breville commented Apr 4, 2018

cc @mirlew

@mirlew
Copy link

mirlew commented Apr 4, 2018

lgtm

@breville breville merged commit 4b8c8f1 into staging Apr 4, 2018
@breville breville deleted the hide-access-report-banner-on-teacher-home branch April 4, 2018 20:50
breville added a commit that referenced this pull request Apr 19, 2018
Hide Professional learning special announcement from /yourschool and teacher /home (though leaving code intact, since I still have aspirations of making this more flag-driven for future years).  Show the access report again on the teacher homepage, essentially reversing #21644.
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

5 participants