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

Fixed issue #09261: emailstatus not filled by default when using remotecontrol invite_participants #223

Merged
merged 1 commit into from Oct 6, 2014

Conversation

Projects
None yet
4 participants
@Shnoulle
Copy link
Collaborator

commented Sep 29, 2014

  • move default value to OK to models BUT : this disallow empty string : is this OK ?
  • for token : use 'caseSensitive'=>false can be done, but only for mySQL
  • add unknow error (from rules) after import token and tracevar the errors
  • fix some other bug with debug>0
Fixed issue #09261: emailstatus not filled by default when using remo…
…tecontrol invite_participants

Dev: move default value to OK to models BUT : this disallow empty string : is this OK ?
Dev: for token : use 'caseSensitive'=>false can be done, but only for mySQL
Dev: add unknow error (from rules) after import token and tracevar the errors
@Shnoulle

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 29, 2014

this disallow empty string : is this OK ? If yes, can redo the patch (or merge)

@maziminke

This comment has been minimized.

Copy link
Collaborator

commented Sep 30, 2014

I don't think this is a good idea. There may be users who simply do not
want to add an email address for whatever reasons. We shouldn't force
them to do so.

Best regards,
Marcel

Am 29.09.2014 um 18:23 schrieb Denis Chenu:

this disallow empty string : is this OK ? If yes, can redo the patch
(or merge)


Reply to this email directly or view it on GitHub
#223 (comment).

@SamMousa

This comment has been minimized.

Copy link
Contributor

commented Sep 30, 2014

There are a lot of very valid reasons to use tokens without email addresses.

Requiring emails will break a lot of integrations.

On Tue, Sep 30, 2014 at 9:16 AM, Marcel Minke notifications@github.com
wrote:

I don't think this is a good idea. There may be users who simply do not
want to add an email address for whatever reasons. We shouldn't force
them to do so.

Best regards,
Marcel

Am 29.09.2014 um 18:23 schrieb Denis Chenu:

this disallow empty string : is this OK ? If yes, can redo the patch
(or merge)


Reply to this email directly or view it on GitHub
<#223 (comment)
.


Reply to this email directly or view it on GitHub
#223 (comment).

@Shnoulle

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 30, 2014

I don't touch to 'email' only to emailstatus.

Empty string move to OK only. User can put [space]/NAOK or anytging else in emailstatus ....

For email : i'm agree : http://bugs.limesurvey.org/view.php?id=9006 I think we must remove the validation from model .

@SamMousa

This comment has been minimized.

Copy link
Contributor

commented Sep 30, 2014

Validation should not be removed.

Instead make the import code validate the model. If there are any validation errors in the email field set the emailstatus to INVALID and svae the model (set validate parameter on the save to false).

@Shnoulle

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 30, 2014

Yes, but this is another thing ....

And actually, there are no way to import email : none (for example)

@Shnoulle

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 30, 2014

@Shnoulle

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 1, 2014

@c-schmitz : do you have a position here ? It's on emailstatus default, not for email or anything else.

The other solution is to do a new filter function : something like 'defaultForNull' , but i think we don't really need it.

@c-schmitz

This comment has been minimized.

Copy link
Contributor

commented Oct 4, 2014

Looks fine to me - please merge.

Shnoulle added a commit that referenced this pull request Oct 6, 2014

Merge pull request #223 from Shnoulle/master_fix09261
Dev : merged Fixed issue #09261: emailstatus not filled by default when using remotecontrol invite_participants

@Shnoulle Shnoulle merged commit 4484e62 into LimeSurvey:master Oct 6, 2014

1 check passed

ci/scrutinizer Scrutinizer: No new issues
Details

@Shnoulle Shnoulle deleted the Shnoulle:master_fix09261 branch Oct 6, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.