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

2nd bugfix for #361 - forgot about register module #393

Merged
merged 3 commits into from Sep 22, 2017

Conversation

chris18890
Copy link
Collaborator

2nd bugfix for #361 - forgot about register module

2nd bugfix for elplatt#361 - forgot about register module
@chris18890
Copy link
Collaborator Author

@ramgarden @elplatt another one to check :)

@@ -27,12 +27,6 @@
*/
function member_add_form () {

// Ensure user is allowed to add members
if (!user_access('member_add')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was this removed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is to allow the form to be rendered without anyone being logged in, so it can be used as part of the register form

)
);
$form['fields'][] = array(
'type' => 'fieldset',
Copy link
Contributor

Choose a reason for hiding this comment

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

You also removed all this membership info?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Instead of trying to maintain 2 duplicate forms, I've set the register form to use the member_add form, as it's identical

Copy link
Contributor

@ramgarden ramgarden left a comment

Choose a reason for hiding this comment

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

Check my inline comments.

@chris18890
Copy link
Collaborator Author

@ramgarden @elplatt replied to the comments, are you happy for this to be merged?

@ramgarden
Copy link
Contributor

Yep. Satisfied my curiosity. Looks OK to me.

@chris18890 chris18890 merged commit 475a051 into elplatt:dev Sep 22, 2017
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

2 participants