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

Fixes #3415 - Quick reply auto-subscribes #3416

Merged
merged 7 commits into from
Sep 6, 2018
Merged

Conversation

AndreRl
Copy link
Contributor

@AndreRl AndreRl commented Aug 25, 2018

Extended from #3342, fixes #3415

Replaces `dont` with an empty string
@AndreRl
Copy link
Contributor Author

AndreRl commented Aug 25, 2018

@euantorano @Eldenroot @effone When you have the time please review if necessary.

@effone
Copy link
Member

effone commented Aug 28, 2018

This is not the desired fix. I placed the method value as dont for a reason. And the reason is this:
https://github.com/mybb/mybb/blob/feature/newreply.php#L835

If you blank out the first value the new reply page will appear with no radio checked instead of first one checked in case of not subscribed:
snap

The desired fix should be keeping the dont as it is and change this line:
https://github.com/mybb/mybb/blob/feature/newreply.php#L382

From
if(!isset($postoptions['subscriptionmethod']))
to
if(!isset($postoptions['subscriptionmethod']) || ($postoptions['subscriptionmethod'] == "dont" && $mybb->get_input('method') == "quickreply"))

@effone effone added the b:1.8 Branch: 1.8.x label Aug 28, 2018
@dvz
Copy link
Member

dvz commented Aug 28, 2018

This above seems to work fine, but it would introduce internal inconsistency (some functions returning dont while the handler expects an empty string, requiring additional processing). Perhaps the first radio input could be checked when the get_subscription_method() output is empty?

@effone
Copy link
Member

effone commented Aug 29, 2018

In that case additional modifications in var naming and templates are required.
I will try to push some changes in the branch (if required).

@effone
Copy link
Member

effone commented Aug 29, 2018

Alright I have made some changes in the PR. Hope its okay now ...

@dvz
Copy link
Member

dvz commented Aug 30, 2018

Looks like the changes of https://github.com/mybb/mybb/pull/3342/files and of this PR still need to be applied to editpost.php.
The template version should be bumped to 1819.

@dvz dvz merged commit 87e1871 into mybb:feature Sep 6, 2018
lairdshaw pushed a commit to lairdshaw/mybb that referenced this pull request Oct 11, 2021
[Rebased for 1.9 by Laird]

* Update functions.php

Replaces `dont` with an empty string

* variable naming change

* spacing

* variable changes in template

* varname change

* subscription method update for edit post

* template version bump
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
b:1.8 Branch: 1.8.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Quick reply auto-subscribes users
3 participants