TWIG

[kicken]

It's a bit pointless to do a try/catch just do die() with the exception message in the catch block. PHP's default behavior for an uncaught exception is to die with the exception message and a stack trace, so just use that. That's the point behind removing the try/catch.

[benanamen]

When you get the default values for your membership fields, you repeat the exact same pair of if statements over and over again with the only difference being the field name.

 

This screams for a loop:

$membership_fields = [
    'membership_category_id',
    'membership_type',
    'membership_type_description',
    'is_promo',
    'is_active',
];

$membership_data = [];
foreach ($membership_fields as $membership_field)
{
    if (!empty($_POST[$membership_field]))
    {
        $membership_data[$membership_field] = $_POST[$membership_field];
    }
    elseif (!empty($row[$membership_field]))
    {
        $membership_data[$membership_field] = $row[$membership_field];
    }
    else
    {
        $membership_data[$membership_field] = '';
    }
}

 

 

Very helpful. Thank you.

 

Isn't it not necessary to create the empty array $membership_data = []; since the array is being created no matter what?

[Jacques1]

PHP creates an array on the fly when you write elements into an undefined variable, yes. But it's a lot more clear both for humans and tools if you explicitly define the array first.

 

For example, PhpStorm cannot infer the type of $a in the following code, and asking for the definiton only gets me to the first array acess in line 4:

<?php

if (true)
    $a['x'] = 2;
else
    $a['x'] = 3;

Adding an explicit array definition fixes both problems:

<?php

$a = [];

if (true)
    $a['x'] = 2;
else
    $a['x'] = 3;

Needless to say, most (if not all) other languages actually require the initial definition.

[benanamen]

The loop you showed in #24 would need to be used numerous times with various pages so I made it into a function as shown here. Any problems doing this?

<?php
function data_source($form_fields)
    {
    $field_data = [];
    foreach ($form_fields as $form_field)
        {
        if (!empty($_POST[$form_field]))
            {
            $field_data[$form_field] = $_POST[$form_field];
            }
        elseif (!empty($row[$form_field]))
            {
            $field_data[$form_field] = $row[$form_field];
            }
        else
            {
            $field_data[$form_field] = '';
            }
        }
    return $field_data;
    }

$_POST['membership_type'] = 'Trial';
$_POST['is_promo'] = 'Yes';

$form_fields = [
    'membership_category_id',
    'membership_type',
    'membership_type_description',
    'is_promo',
    'is_active',
];

$arr = data_source($form_fields) ;
?>

And then send the array to the template.....

echo $twig->render('template.twig', ['result' => $arr]);        

[Jacques1]

The function doesn't have access to any $row variable, you need an extra parameter for this.

 

Actually, I wonder if the logic even make sense. Right now, posting an empty field resets the field to the value from the database, which is definitely not what I expect as a user. If an empty value isn't permitted, the application should say so instead of silently altering my input.

 

So I'd rather go with three strictly separated cases:

• When the user has posted data, all field values are taken from $_POST (with error messages when needed). A database query isn't necessary.
• If the user edits the entity for the first time, all values are taken from the database.
• If the user adds a new entity, all fields are empty.

 

 

[benanamen]

Where you have the empty set in the else, the original template had a ternary to set to empty in the form field instead so really the else should not be there in which case the logic would work as it has been. I had just not tested it in the app yet to notice that issue.

[benanamen]

 

So I'd rather go with three strictly separated cases:

• When the user has posted data, all field values are taken from $_POST (with error messages when needed). A database query isn't necessary.
• If the user edits the entity for the first time, all values are taken from the database.
• If the user adds a new entity, all fields are empty.

 

If you look at post #20, i do have it as 3 strictly separated cases there and that is also how the original non-twig template is. Only difference that will be now, is you showed me a cleaner option to not repeat all the if statements and by putting it to a function, will tremendously reduce the code in the app.

 

So this updated function along with a ternary in the template field to = ' '  as in value="{{ membership_type ?? '' }}" should do it right?

function data_source($form_fields)
    {
    $field_data = [];
    foreach ($form_fields as $form_field)
        {
        if (!empty($_POST[$form_field]))
            {
            $field_data[$form_field] = $_POST[$form_field];
            }
        if (!empty($row[$form_field]))
            {
            $field_data[$form_field] = $row[$form_field];
            }
        }
    return $field_data;
    }

Edited August 14, 2016 by benanamen

[kicken]

So this updated function along with a ternary in the template field to = ' '  as in value="{{ membership_type ?? '' }}" should do it right?

The function still does not have access to the $row variable, you need to pass that to the function. You also have not addressed the issue of the field being reset to the database value when a user posts it as empty during editing, unless you just don't pass any original values into the function in that case.

 

function extractData($fields, $source)
{
	$data = [];
	foreach ($fields as $name)
	{
		if (!empty($source[$name]))
		{
			$data[$name] = $source[$name];
		}
	}
	return $data;
}

$Defaults = [];
if ($_SERVER['REQUEST_METHOD'] == 'POST'){
    $fields = ['membership_category_id', ...];
    $Defaults = extractData($fields, $_POST);
} else if ($IsEditingMode){
    $Defaults = loadRecordFromDatabase();
}

Defaults will be an empty array initially. If data was posted, you extract the desired fields from $_POST ignoring empty fields, otherwise if the user is editing a record you load the defaults from the database.

 

In the twig there's no need a condition, just output the value of the defaults array for that field.

[benanamen]

 

The function still does not have access to the $row variable,

 

Yeah, I forgot about that. I was posting from my phone away from my DEV setup. It seems I should have done my twig testing inside the app. I would have seen that right away.

 

 

 

You also have not addressed the issue of the field being reset to the database value when a user posts it as empty during editing

 

Funny you mentioned that. That is something i noticed yesterday while I was testing. I checked the app and it has the same problem. Never noticed it before since the fields I had been dealing with were all required fields. Back to the drawing board!

Edited August 14, 2016 by benanamen

[Jacques1]

@benanamen:

 

I think I understand now where the problem is and why we've been talking past each other:

 

You seem to already load the initial form with a POST request, because there are no URL parameters for the ID (only $_POST['id']). That would explain why you need this strange if-empty-then-load-from-database logic for each individual field: You need it to prefill the initial form.

 

Unfortunaly, this logic doesn't work in subsequent requests, because as kicken and I already said, you'll override fields which the user has left empty.

 

To fix this, put the ID into a URL parameter. Then you can actually distinguish between the different cases like in #33:

• when in edit mode with a GET request, load values from database
• when in edit mode with a POST request, take values from POST parameters
• when in add mode, use empty values

Edited August 14, 2016 by Jacques1

[benanamen]

You seem to already load the initial form with a POST request, because there are no URL parameters for the ID (only $_POST['id']). 

 

EXACTLY!

 

For edit mode, we start off with a table list of records which also includes an edit button on each row. That button POST's the ID which is used to query for the edit data. 

 

 

 

Unfortunaly, this logic doesn't work in subsequent requests, because as kicken and I already said, you'll override fields which the user has left empty.

 

 

That is the exact problem I am now seeing. I never noticed the problem before.

 

 

To fix this, put the ID into a URL parameter. Then you can actually distinguish between the different cases like in #33:

• when in edit mode with a GET request, load values from database
• when in edit mode with a POST request, take values from POST parameters
• when in add mode, use empty values

 

So the edit button should GET instead of POST if I am understanding correctly.

 

 

So in the function would I then be using if

 

($_SERVER['REQUEST_METHOD'] == 'POST')

 

AND

 

if ($_SERVER['REQUEST_METHOD'] == 'GET'){

 

?

 

From a clean slate, no code, what is the optimum way to use a single twig template for add and edit? That's really the bottom line. What I was doing is obviously not right.

Edited August 14, 2016 by benanamen

[Jacques1]

<?php

if ($_SERVER['REQUEST_METHOD'] == 'POST')    // update or add record
{
    if (isset($_GET['id']))
    {
        // @TODO update record
    }
    else
    {
        // @TODO add new record
    }

    $membership_data = $_POST;
}
elseif (isset($_GET['id']))    // initial form for editing record
{
    $membership_data = load_membership_data($_GET['id']);
}
else    // initial form for adding record
{
    $membership_data = [];
}

// @TODO pass $membership_data to template

<form method="post" action="{{ {'id': get.id}|url_encode }}">
    <input type="text" name="membership_type_description" value="{{ membership_data.membership_type_description ?? '' }}">
    ...
</form>

So the edit button should GET instead of POST if I am understanding correctly.

 

Yes (or a simple link).

[benanamen]

Thank you.

Starting from a blank page I used the code from #37. The problem of overriding fields is fixed and now  you can Insert and Update from a single twig template. Additionally, you can also insert and update from a single php file, something I didn't have before.

 

The problem I now have is with the dropdown. The dropdown is used for both the insert and update. Currently the query to get that data is part of the update record code. A new record is not being updated, so how do I best go about handling it?

 

I also see that you show a function to get the update data. I assume that is a better way to go about getting the data. I will come to that when I get everything else sorted out. ($membership_data = load_membership_data($_GET['id']);)

 

Aside from what is mentioned, any other issues at this point?

 

The part for the dropdown. Where would this go now?

$options = array();
foreach ($stmt as $row2)
    {
    $options[] = [
        'selected' => isset($membership_category_id) && $membership_category_id == $row2['membership_category_id'],
        'value' => $row2['membership_category_id'],
        'label' => $row2['membership_category']
    ];
    }

Current version of pre-template code

<?php
// TODO: Change arrays to new syntax []

$error = array();

if ($_SERVER['REQUEST_METHOD'] == 'POST') // update or add record
    {
    //------------------------------------------------------------------------------------
    // Trim Data, Check Missing Fields
    //------------------------------------------------------------------------------------

    include './includes/validate_membership_types.php';

    //------------------------------------------------------------------------------------
    // Check for errors
    //------------------------------------------------------------------------------------

    if ($error)
        {
        show_form_errors($error);
        }

    //------------------------------------------------------------------------------------
    // Update Data
    //------------------------------------------------------------------------------------

    elseif (isset($_GET['id']))
        {
        die('Record Updated'); //TODO: Temp Dev
        $sql  = "UPDATE membership_types SET membership_category_id =?, membership_type =?, membership_type_description =?, is_promo =?, is_active =? WHERE membership_type_id=?";
        $stmt = $pdo->prepare($sql);
        $stmt->execute(array(
            $_POST['membership_category_id'],
            $_POST['membership_type'],
            $_POST['membership_type_description'],
            $_POST['is_promo'],
            $_POST['is_active'],
            $_POST['id']
        ));
        die(header("Location: {$_SERVER['SCRIPT_NAME']}?p=list_membership_types&edit"));
        }

    //------------------------------------------------------------------------------------
    // Insert Data
    //------------------------------------------------------------------------------------

    else
        {
        // @TODO add new record
        /*$sql  = "INSERT INTO membership_types (membership_category_id, membership_type, membership_type_description, is_promo, is_active) VALUES (?, ?, ?, ?, ?)";
        $stmt = $pdo->prepare($sql);
        $stmt->execute(array(
        $_POST['membership_category_id'],
        $_POST['membership_type'],
        $_POST['membership_type_description'],
        $_POST['is_promo'],
        $_POST['is_active']
        ));
        die(header("Location: {$_SERVER['SCRIPT_NAME']}?p=list_membership_types&insert"));*/
        die('Record Added'); // Temp Dev
        }

    $membership_data = $_POST;

    }

//------------------------------------------------------------------------------------
// Initial form for editing record
//------------------------------------------------------------------------------------

elseif (isset($_GET['id']))
    {
    //$membership_data = load_membership_data($_GET['id']);
    $sql  = "SELECT membership_category_id, membership_type, membership_type_description, is_promo, is_active FROM membership_types WHERE membership_type_id=?";
    $stmt = $pdo->prepare($sql);
    $stmt->execute(array(
        $_GET['id']
    ));
    $membership_data = $stmt->fetch(PDO::FETCH_ASSOC);
    }

//------------------------------------------------------------------------------------
// Initial form for editing record
//------------------------------------------------------------------------------------

else
    {
    $membership_data = array();
    }

//------------------------------------------------------------------------------------
// Pass $membership_data to template
//------------------------------------------------------------------------------------

echo $twig->render('form_membership_types.twig', array(
    'action' => 'Edit',
    'form_action' => $_SERVER['SCRIPT_NAME'],
    'get' => $_GET,
    'data' => $membership_data,
    'error' => $error
));
?>

Edited August 14, 2016 by benanamen

[Jacques1]

The problem I now have is with the dropdown. The dropdown is used for both the insert and update. Currently the query to get that data is part of the update record code. A new record is not being updated, so how do I best go about handling it?

 

There's no need for special code for the dropdown. Simply pass all membership categories to the template:

'membership_categories' => $pdo->query('SELECT membership_category_id, membership_category FROM membership_category')->fetchAll()

Building the dropdown menu can be done in the template. To preselect an option, check the defaults array:

{% if membership_data.membership_category_id is defined and category.membership_category_id == membership_data.membership_category_id %}selected{% endif %}

Aside from what is mentioned, any other issues at this point?

 

Nothing terribly important.

• You should avoid using SCRIPT_FILENAME for form actions, because it's unnecessary and assumes that the public URL matches the physical filename (which is not true when you start rewriting URLs). Relative URLs like in #37 are much more convenient and robust. For example, “?id=12” is automatically resolved to the right path.
• The validation which happens in the included script and “magically” updates the $errors array is confusing. A function would be better.

Edited August 14, 2016 by Jacques1

[benanamen]

What would you suggest for a function for the missing required fields? The validation include is just simple ifs that add errors to the error array if there is a missing field. The validation currently is just for missing required fields and a function call converting certain empty fields to null.

[kicken]

If you want something generic, you could pass in a map of fields and error messages to a function that checks if they are empty or not and returns a list of errors.

function validateRequired($fields, $data){
    $Errors = [];
    foreach ($fields as $name=>$message){
        if (!isset($data[$name]) || trim($data[$name]) == ''){
            $Errors[] = $message;
        }
    }

    return $Errors;
}

$required = [
    'membership_type' => 'Type name is required'
    , 'membership_type_description' => 'Description is required'
    //, ...
];

$Errors = validateRequired($required, $_POST);
if (count($Errors) == 0){
    //Valid
}

If you need something a bit more tailored to a specific form, you can wrap that in it's own validate function which might call the required one as part of it's work.

 

function validateMembershipType($data){
    $required = [
        'membership_type' => 'Type name is required'
        , 'membership_type_description' => 'Description is required'
        //, ...
    ];

    $Errors = validateRequired($required, $data);
    if (count($Errors) == 0){
        if ($data['is_promo']){
            $required = ['promo_start' => 'Promo start date is required'];
            $Errors = array_merge($Errors, validateRequired($required, $data));
            if (!empty($data['promo_end'])){
                $start = DateTime::createFromFormat('m/d/Y');
                $end = DateTime::createFromFormat('m/d/Y');
                if ($end <= $start){
                    $Errors[] = 'Promo end date must be after promo start date';
                }
            }
        }
    }

    return $Errors;
}

That is more or less how I handle it in one of the older systems I maintain. In that system a typical CRUD style page looks like this (in pseudo-code):

<?php

$action = isset($_GET['action'])?$_GET['action']:'list';
$id = isset($_GET['id'])?$id:null;
switch ($action) {
    case 'add':
        DoModify('add');
        break;
    case 'edit':
        if (!$id){
            throw new \RuntimeException('Missing id parameter');
        }
        DoModify('edit', $id);
        break;
    case 'delete':
        if (!$id){
            throw new \RuntimeException('Missing id parameter');
        }
        DoDelete($id);
        break;
    default:
        DoList();
}

function DoList(){
    $items = loadAllFromDB();
    Template::render('list.tpl', ['items' => $items]);
}

function DoModify($action, $id=null){
    $Defaults = $Errors = [];
    if ($_SERVER['REQUEST_METHOD'] == 'POST'){
        $Defaults = $data = $_POST;
        if (validatePostData($action, $data, $Errors)){
            if ($id){
                updateRecord($id, $data);
            } else {
                createRecord($data);
            }
            RedirectTo('?action=list');
            return;
        }
    } else if ($id){
        $Defaults = loadSingleFromDB($id);
    }

    Template::render('modify.tpl', [
        'id' => $id
        , 'action' => $action
        , 'Defaults' => $Defaults
        , 'Errors' => $Errors
    ]);
}

function DoDelete($id){
    if ($_SERVER['REQUEST_METHOD'] == 'POST'){
        //Create and run delete query
    }

    RedirectTo('?action=list');
}

function validatePostData($type, &$data, &$Errors){
    //validation and normalization here

    return count($Errors) == 0;
}

function updateRecord($id, $data){
    //Create and run update query
}

function createRecord($data){
    //Create and run insert query
}

I cheat a little bit and let the validation function also normalize the data (convert dates to DateTime objects, empty fields to null, etc). As such the data is pass-by-reference so that the normalization is reflected in the main code also. Errors are gathered in a pass-by-reference variable also so the function can return a simple true/false based on the overall validation. Edited August 15, 2016 by kicken

[benanamen]

<form method="post" action="{{ {'id': get.id}|url_encode }}">

You should avoid using SCRIPT_FILENAME for form actions, because it's unnecessary and assumes that the public URL matches the physical filename (which is not true when you start rewriting URLs). Relative URLs like in #37 are much more convenient and robust. For example, “?id=12” is automatically resolved to the right path.

 

@Jaques1,

 

This does not work as is.

 

This is a working url:

http://localhost/mydev/index.php?p=edit_newdev&id=1

 

Doing what you posted results in

<form class="form-horizontal" method="post" action="id=1">

 

To the URL

http://localhost/mydev/id=1

Edited August 15, 2016 by benanamen

[Jacques1]

Add the missing question mark.

<form method="post" action="?{{ {'id': get.id}|url_encode }}">
                            ^

Edited August 15, 2016 by Jacques1

[benanamen]

Still not right. It needs the GET p page name value. On submit goes to http://localhost/mydev/index.php?id=1

 

This works:

<form class="form-horizontal" action="{{ form_action }}?p={{ get.p }}{% if get.id %}&id={{ get.id }}{% endif %} " method="post">

Is there an improvement to that?

 

FYI:

'form_action' => $_SERVER['SCRIPT_NAME'], 

Edited August 15, 2016 by benanamen

[Jacques1]

Still not right.

 

It's example code, and I thought it's obvious how to insert different or additional parameters.

<form method="post" action="?{{ {'id': get.id, 'p': get.p}|url_encode }}">

Alternatively, you could just copy the entire URL query (i. e. $_SERVER['QUERY_STRING']), but then you may copy garbage data as well.

 

 

 

FYI:

'form_action' => $_SERVER['SCRIPT_NAME'], 

 

?

[benanamen]

Ok, got it. When it comes to doing something in twig I don't know what should be obvious yet. You have already shown me several twig things that were not obvious to me.

 

I thought  ?{{ {'id': get.id}|url_encode }}  was some sort of dynamic twig thing to get the whole url replacing $_SERVER['SCRIPT_NAME']. I pretty much have the jist of using TWIG with mysql now thanks to you and kicken. It was only 4 days ago that I even really looked at twig.

 

So here is the (hopefully) final version of the template and pre-template code. I am sure there is probably some fine tuning that can be done somewhere. Any further comments or improvements welcome.

 

Current Template

<form class="form-horizontal" action="{{ form_action }}?{{ { 'p': get.p, 'id': get.id }|url_encode }}" method="post">

<fieldset>
<legend>
{% if get.action == 'add' %}Add{% endif %}{% if get.action == 'edit' %}Edit{% endif %} Membership Types
</legend>

<!-- Select Basic -->
<div class="form-group {{ error.membership_category_id ? 'has-error' : '' }}">
   <label class="col-md-4 control-label" for="membership_category_id">Membership Category <span style="color: #FF0000;">*</span></label>
   <div class="col-md-4">
      <select id="membership_category_id" name="membership_category_id" class="form-control">
         <option value="" style="display:none">Select Category</option>
         {% for option in category %}
         <option value="{{ option.membership_category_id }}" {% if form_data.membership_category_id is defined and option.membership_category_id == form_data.membership_category_id %}selected{% endif %}>{{ option.membership_category }}</option>
         {% endfor %}
      </select>
   </div>
</div>

<!-- Text input-->
<div class="form-group {{ error.membership_type ? 'has-error' : '' }}">
   <label class="col-md-4 control-label" for="membership_type">Membership Type <span style="color: #FF0000;">*</span></label>
   <div class="col-md-4">
      <input id="membership_type" name="membership_type" type="text" placeholder="Membership Type" class="form-control input-md" value="{{ form_data.membership_type ?? '' }}">
   </div>
</div>

<!-- Text input-->
<div class="form-group  {{ error.membership_type_description ? 'has-error' : '' }}">
   <label class="col-md-4 control-label" for="membership_type_description">Membership Type Description <span style="color: #FF0000;">*</span></label>
   <div class="col-md-4">
      <input id="membership_type_description" name="membership_type_description" type="text" placeholder="Membership Type Description" class="form-control input-md" value="{{ form_data.membership_type_description ?? '' }}">
   </div>
</div>

<div class="form-group">
  <label class="col-md-4 control-label"></label>
  <div class="col-md-4">
    <label class="checkbox-inline" for="is_promo">
      <input type="checkbox" name="is_promo" id="is_promo" value="1" {% if form_data.is_promo %}checked{% endif %} >
      Promotional
    </label>
    <label class="checkbox-inline" for="is_active">
      <input type="checkbox" name="is_active" id="is_active" value="1" {% if form_data.is_promo %}checked{% endif %}  >
      Active
    </label>
  </div>
</div>

<div class="form-group">
   <div class="col-md-offset-4 col-md-4">
    {% if get.id is not null %}<input type="hidden" name="id" value="{{ get.id }}">{% endif %}
    <input type="submit" name="submit" value="Submit" class="btn btn-primary">
   </div>
</div>
</fieldset>
</form>

<?php
$error = [];

if ($_SERVER['REQUEST_METHOD'] == 'POST') // update or add record
    {
    //------------------------------------------------------------------------------------
    // Trim Data, Check Missing Fields
    //------------------------------------------------------------------------------------

    include './includes/validate_membership_types.php';

    //------------------------------------------------------------------------------------
    // Check for errors
    //------------------------------------------------------------------------------------

    if ($error)
        {
        show_form_errors($error);
        }

    //------------------------------------------------------------------------------------
    // Update Data
    //------------------------------------------------------------------------------------

    elseif (isset($_GET['id']))
        {
        $sql  = "UPDATE membership_types SET membership_category_id =?, membership_type =?, membership_type_description =?, is_promo =?, is_active =? WHERE membership_type_id=?";
        $stmt = $pdo->prepare($sql);
        $stmt->execute(array(
            $_POST['membership_category_id'],
            $_POST['membership_type'],
            $_POST['membership_type_description'],
            $_POST['is_promo'],
            $_POST['is_active'],
            $_POST['id']
        ));
        die(header("Location: {$_SERVER['SCRIPT_NAME']}?p=list_membership_types&edit"));
        }

    //------------------------------------------------------------------------------------
    // Insert Data
    //------------------------------------------------------------------------------------

    else
        {
        $sql  = "INSERT INTO membership_types (membership_category_id, membership_type, membership_type_description, is_promo, is_active) VALUES (?, ?, ?, ?, ?)";
        $stmt = $pdo->prepare($sql);
        $stmt->execute(array(
        $_POST['membership_category_id'],
        $_POST['membership_type'],
        $_POST['membership_type_description'],
        $_POST['is_promo'],
        $_POST['is_active']
        ));
        die(header("Location: {$_SERVER['SCRIPT_NAME']}?p=list_membership_types&insert"));
        }

    $form_data = $_POST;
    }

//------------------------------------------------------------------------------------
// Initial form for editing record
//------------------------------------------------------------------------------------

elseif (isset($_GET['id']))
    {
    $sql  = "SELECT membership_category_id, membership_type, membership_type_description, is_promo, is_active FROM membership_types WHERE membership_type_id=?";
    $stmt = $pdo->prepare($sql);
    $stmt->execute(array(
        $_GET['id']
    ));
    $form_data = $stmt->fetch(PDO::FETCH_ASSOC);
    }

//------------------------------------------------------------------------------------
// Initial form for adding record
//------------------------------------------------------------------------------------

else
    {
    $form_data = array();
    }

//------------------------------------------------------------------------------------
// Pass $form_data to template
//------------------------------------------------------------------------------------

echo $twig->render('form_membership_types.twig', [
    'form_action' => $_SERVER['SCRIPT_NAME'],
    'get' => $_GET,
    'form_data' => $form_data,
    'error' => $error,
    'category' => $pdo->query('SELECT membership_category_id, membership_category FROM membership_category')->fetchAll(),
]);
?>

Edited August 15, 2016 by benanamen