FEATURE: opt-in guidance on topics for users without access (PR #7852)

Screenshot 2019-07-03 at 17 31 47

https://github.com/discourse/discourse/pull/7521

GitHub

You’ve signed the CLA, jjaffeux. Thank you! This pull request is ready for review.

Is there a related discussion for this? Some context would be nice.

Could we get the URL from document.location instead since it’s not private APIs?

Is there a related discussion for this? Some context would be nice.

This is basically https://github.com/discourse/discourse/pull/7521 but I took ownership of it

The PR has mostly been reviewed and accepted but I got to fix few things and merge conflicts

Overall it seems like a great feature but I think it needs some tweaks.

    params.require(:reason) if params[:topic_id].blank?

This will raise an exception if that topic_id does not exist won’t it?

      group_request_sent: "Your group membership request has been sent. You will be informed when it's accepted."

Should it say the group name here? Makes it extra clear to the approver.

I guess I would write this way:

if params[:topic_id] && topic = Topic.find_by_id(params[:topic_id])
  reason = I18n.t("groups.view_hidden_topic_request_reason", topic_url: topic.url)
end

Yes the present? check is fine, I was more worried if the id was 10123012031203 and no topic could be returned.

This pull request has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/show-group-opt-in-guidance-on-topics-you-have-no-access-to/110195/9