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

Update PersonEditor.php #3344

Closed
wants to merge 2 commits into from
Closed

Update PersonEditor.php #3344

wants to merge 2 commits into from

Conversation

bdadieb
Copy link

@bdadieb bdadieb commented Nov 17, 2017

  • adding more details in person data
  • adding icons to fields

personeditor

Copy link
Contributor

@crossan007 crossan007 left a comment

Choose a reason for hiding this comment

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

changes to the schema need to be in line with recommendations here: https://github.com/ChurchCRM/CRM/wiki/Code-Functional-Reference#changes-to-the-db-schema

@bdadieb
Copy link
Author

bdadieb commented Nov 18, 2017

@crossan007 thanks for your review
I believe that schema.xml could be updated but in the mean time i don't know the regular steps to do that (Updating some fields in Churchcrm)

Copy link
Contributor

@DawoudIO DawoudIO left a comment

Choose a reason for hiding this comment

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

Great 1st PR, some questions, and simple code cleanup. Also missing DB scripts to add DB columns.

see #3323 for an example of how to add db changes.

@@ -5,7 +5,12 @@
* website : http://www.churchcrm.io
* copyright : Copyright 2001, 2002, 2003 Deane Barker, Chris Gebhardt
* Copyright 2004-2005 Michael Wilt
*
*
* ChurchCRM is free software; you can redistribute it and/or modify
Copy link
Contributor

Choose a reason for hiding this comment

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

we removed all of these out of the system...

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for your reviews
i was working on an elder version 👍

$sNationalError = gettext('Entered ID is too big,<p>Please enter only the "14" Digits National ID.');
$bErrorFlag = true;
} elseif (strlen($iNational) < 14) {
$sNationalError = gettext('You entered too small ID,<p>Please enter only the "14" Digits National ID.');
Copy link
Contributor

Choose a reason for hiding this comment

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

no html in messages, please break into two strings... also in the US it is called Social Security Number. Since US English is the default language others can override in the locale settings.

Copy link
Author

Choose a reason for hiding this comment

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

ok

$sNationalError = gettext('You entered too small ID,<p>Please enter only the "14" Digits National ID.');
$bErrorFlag = true;
} elseif (strlen($iNational) < 1) {
$sNationalError = gettext('You must enter the "14" Digits National ID.');
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

sure I'll follow

$sWorkplaceError = gettext('You must enter Company or Workplace.');
$bErrorFlag = true;
} else {
$sWorkplace = $sWorkplace;
Copy link
Contributor

Choose a reason for hiding this comment

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

not needed

$sWorkError = gettext('You must enter the Job or Work description.');
$bErrorFlag = true;
} else {
$sWork = $sWork;
Copy link
Contributor

Choose a reason for hiding this comment

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

not needed

<p/>
<div class="row">
<div class="col-md-4">
<label for="RecChurch"><?= gettext('Church of Recognition') ?>:</label>
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the goal here... we are building a new system for having a Diocese / Parish

Copy link
Author

Choose a reason for hiding this comment

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

great but this is not for that its for confession follow-up
also i need your help to set alert for last confession date if it overdue number of days

</div>
</div>
<div class="col-md-3">
<label for="RecFather"><?= gettext('Recognition Father') ?>:</label>
Copy link
Contributor

Choose a reason for hiding this comment

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

is this father of confession if so I'm working on another system for it.

Copy link
Author

Choose a reason for hiding this comment

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

yes

</div>
</div>
<div class="form-group col-md-3 col-lg-3">
<label><?= gettext('Last Recognition') ?>:</label>
Copy link
Contributor

Choose a reason for hiding this comment

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

same.

value="<?= change_date_for_place_holder($dMembershipDate) ?>" maxlength="10" id="sel1" size="11"
placeholder="<?= SystemConfig::getValue("sDatePickerPlaceHolder") ?>">
value="<?= $dMembershipDate ?>" maxlength="10" id="sel1" size="11"
placeholder="YYYY-MM-DD">
Copy link
Contributor

Choose a reason for hiding this comment

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

please put back the system setting as it affects how people format dates

Copy link
Author

Choose a reason for hiding this comment

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

ok

value="<?= change_date_for_place_holder($dFriendDate) ?>" maxlength="10" id="sel2" size="10"
placeholder="<?= SystemConfig::getValue("sDatePickerPlaceHolder") ?>">
value="<?= $dFriendDate ?>" maxlength="10" id="sel2" size="10"
placeholder="YYYY-MM-DD">
Copy link
Contributor

Choose a reason for hiding this comment

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

same

@ghost ghost assigned DawoudIO Nov 18, 2017
@ghost ghost added the In Review label Nov 18, 2017
@DawoudIO DawoudIO added this to the On-Deck milestone Nov 28, 2017
@DawoudIO
Copy link
Contributor

DawoudIO commented Dec 3, 2017

please fix merge issues

@lbridgman lbridgman added this to In Progress in Bug fix pipeline Dec 4, 2017
@lbridgman lbridgman moved this from In Progress to In Review in Bug fix pipeline Dec 4, 2017
@lbridgman lbridgman moved this from In Review to In Progress in Bug fix pipeline Dec 4, 2017
@lbridgman lbridgman removed this from In Progress in Bug fix pipeline Dec 5, 2017
@DawoudIO
Copy link
Contributor

please reopen when code is ready

@DawoudIO DawoudIO closed this Feb 19, 2018
@ghost ghost removed the In Review label Feb 19, 2018
@DawoudIO DawoudIO removed this from the On-Deck milestone Feb 19, 2018
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