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

Issue #361 emergency details move #388

Merged
merged 15 commits into from Sep 18, 2017

Conversation

chris18890
Copy link
Collaborator

moves emergency details from contact module to member module, closes issue #361

redone PR #362 to factor in new pull requests

@@ -24,7 +24,7 @@
$crm_version = array(
'major' => 0
, 'minor' => 4
, 'patch' => 6
, 'patch' => 7
Copy link
Contributor

Choose a reason for hiding this comment

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

Would this be a big enough change to warrant moving up the minor version number by one?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

probably, yes, though @elplatt had previously mentioned that because the project was in maintenance only mode rather than active development, & it was a DB change, that he wasn't happy pushing this change


$member_data = crm_get_data('member', array('cid'=>$_POST['cid']));
$member = $member_data[0]['member'];
$esc_cid = mysqli_real_escape_string($db_connect, $_POST['cid']);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this esc_cid get set up at the top of this scope then used throughout instead of $_POST['cid'] ??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Probably! this code has been sitting around for over a year so I'd need to tweak & test it again!

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.

Left a few inline comments. See if they make any sense or if I'm off my rocker.

@chris18890
Copy link
Collaborator Author

Fixed a bug that meant that escaping values wasn't working, cheers for the heads up :)

@chris18890 chris18890 merged commit d8ed68f into elplatt:dev Sep 18, 2017
@chris18890 chris18890 deleted the issue361_emergency_details_move branch September 18, 2017 14:48
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

3 participants