JavaScript ajax formquestion editor

Question

This is the JavaScript file that i use for my editor that allows people to edit questions from a form.

I'm wondering if this sort of approach to use JavaScript objects is good or that I kind of miss the point here?

$(document).ready(function() {
    manager = new Customerformquestionmanager();
    manager.formId = firstFormId;
    manager.getCategorys();


    $('#formSelect').change(function() {
        manager.getCategorys();
    });

    $('#categorySelect').change(function() {
        manager.categoryId = $(this).val();
        manager.getQuestions();
    });

    $('.dataTable').on('click', '.editBtn', function() {
        parent = $(this).parent();
        id = parent.find('.rowId').html();
        question = parent.find('.rowQuestion').html();
        type = parent.find('.rowType').html();
        answers = parent.find('.rowAnswer').html();
        customType = parent.find('.rowCustomType').html();


        $('#editId').val(id)
        $('#editQuestion').val(question);
        $('#editType').val(type);
        $('#editAnswers').val(answers);
        $('#editCustomType').val(customType);
    });

    $('#saveBtn').click(function() {
        questionObj = {
            question: $('#editQuestion').val(),
            type: $('#editType').val(),
            custom_answers: $('#editAnswers').val(),
            custom_type: $('#editCustomType').val()
        };

        if($('#editId').val() != '') {
            questionObj.id = $('#editId').val();
        }

        manager.saveQuestion(questionObj);
    });
});

function Customerformquestionmanager() {
    this.formId = false;
    this.currentQuestionId = false;
    this.categoryId = false;
    this.categorySelect = $('#categorySelect');
    this.dataTable = $('.datatTable');
    this.handler = '/handlers/backend/customerformhandler';
}

Customerformquestionmanager.prototype.getCategorys = function() {
    if(this.formId) {
        obj = this;
        $.ajax({
            'url': this.handler,
            'type': 'POST',
            'data': {
                'method': 'getcategorysbyformid',
                'id': this.formId
            }
        }).done(function(data) {
            if(typeof data != 'undefined')
                data = JSON.parse(data);
            if(typeof data != 'undefined') {
                obj.categorySelect.find('option').remove();

                length = data.length;
                for(i = 0; i < length; i++) {
                    selected = '';
                    if(i == 0) {
                        obj.categoryId = data[0].id;
                        selected = 'selected="selected"';
                        obj.getQuestions();
                    }
                    obj.categorySelect.append('<option' + selected + ' value="' + data[i].id + '">' + data[i].title + ' (id: ' + data[i].id + ')</option>');
                }
            }
        });
    }
}

Customerformquestionmanager.prototype.getQuestions = function() {
    if(this.categoryId) {
        obj = this;
        $.ajax({
            'url': this.handler,
            'type': 'POST',
            'data': {
                'method': 'getquestionsbycategoryid',
                'id': this.categoryId
            }
        }).done(function(data) {
            obj.dataTable.find('tr').not(':first').remove();
            if(typeof data !== 'undefined' && data) 
                data = JSON.parse(data);
            if(typeof data !== 'undefined') {
                length = data.length;
                for(i = 0; i < length; i++) {
                    obj.dataTable.append(
                        '<tr id="category' + data[i].id + '">' +
                            '<td class="rowId">' + data[i].id + '</td>' +
                            '<td class="rowQuestion">' + data[i].question + '</td>' +
                            '<td class="rowType">' + data[i].type + '</td>' +
                            '<td class="rowAnswer">' + data[i].custom_answers + '</td>' +
                            '<td class="rowCustomType">' + data[i].custom_type + '</td>' +
                            '<td class="editBtn">edit</td>' +
                            '<td class="redClickable">delete</td>' +
                        '</tr>'
                    );
                }
            }
        });
    }
}

Customerformquestionmanager.prototype.saveQuestion = function(dataobj) {
    if(typeof dataobj != 'undefined') {
        dataobj.method = 'savequestion';
        dataobj.category_id = this.categoryId;
        obj = this;
        if(typeof dataobj.id !== 'undefined')
            this.currentQuestionId = dataobj.id;
        $.ajax({
            'url': this.handler,
            'type': 'POST',
            'data': dataobj
        }).done(function(data) {
            if(typeof data !== 'undefined' && data) {
                data = JSON.parse(data);
                if(typeof data !== 'undefined') {
                    obj.dataTable.append(
                        '<tr id="category' + data.id + '">' +
                            '<td class="rowId">' + data.id + '</td>' +
                            '<td class="rowQuestion">' + dataobj.question + '</td>' +
                            '<td class="rowType">' + dataobj.type + '</td>' +
                            '<td class="rowAnswer">' + dataobj.custom_answers + '</td>' +
                            '<td class="rowCustomType">' + dataobj.custom_type + '</td>' +
                            '<td class="editBtn">edit</td>' +
                            '<td class="redClickable">delete</td>' +
                        '</tr>'
                    );
                    alert('question added');
                }
            }
            else {
                $('category' + obj.currentQuestionId + ' .rowQuestion').html(dataobj.question);
                $('category' + obj.currentQuestionId + ' .rowType').html(dataobj.type);
                $('category' + obj.currentQuestionId + ' .rowAnswer').html(dataobj.answers);
                $('category' + obj.currentQuestionId + ' .rowCustomType').html(dataobj.custom_type);                

                alert('question saved');
            }

        });
    }
}

This is the PHP/HTML template that it works with (not intended to be the focus of this review):

<?php 
    require_once __DIR__ . '/../classes/database.php';
    require_once __DIR__ . '/../classes/security.php';
    require_once __DIR__ . '/../includes/checkauth.php';
    require_once __DIR__ . '/../includes/backendincludes.php';

    $db = Database::getInstance();
    $forms = $db->getAsArray('SELECT id, title FROM ea_customer_form');
    $lastForm = end($forms);
    $lastForm = $lastForm['id'];
    reset($forms); 
?>
<script type="text/javascript" src="/js/backend/customerformquestion.js"></script>
<script type="text/javascript">var firstFormId = <?php echo $lastForm; ?>;</script>
<div id="maincontent">
    Select form:
    <br />
    <select id="formSelect">        
        <?php 
            foreach($forms as $form) {
                echo '<option value="' . $form['id'] . '">' . $form['title'] . '</option>';
            }
        ?>
    </select>
    <br /><br />

    Select category in form:<br />

    <select id="categorySelect"></select>

    <br /><br />    
    <input type="button" value="clear fields (back to add new)" id="clearBtn">

    <br /><br /><hr><br />

    Required:
    <table>
        <tr>
            <td>Id: </td>
            <td><input type="text" id="editId" readonly="readonly"></td>
        </tr>
        <tr>
            <td>Question: </td>
            <td><input type="text" id="editQuestion" style="width: 600px;"></td>
        </tr>
        <tr>
            <td>Type: </td>
            <td>
                <select id="editType">
                    <option value="textarea">Text area</option>
                    <option value="radio">Radio</option>
                </select>
            </td>
        </tr>
    </table>
    <br /><br />
    Optional: 
    <table>
        <tr>
            <td>Answers: </td>
            <td><input type="text" id="editAnswers" style="width: 600px;" placeholder="answers comma seperated (for example: yes,no,maybe)"></td>
        </tr>
        <tr>
            <td>Custom type:</td>
            <td>
                <select id="editCustomType">
                    <option value="">none</option>
                    <option value="grouped">Grouped</option>
                </select>
            </td>
        </tr>
        <tr>
            <td><input type="button" value="save" id="saveBtn"></td>
        </tr>
    </table>

    <br /><br /><hr><br />

    <table class="dataTable">
        <tr>
            <th>Id</th>
            <th>Question</th>
            <th>Type</th>
            <th>Answers</th>
            <th>Custom type</th>
            <th>Edit</th>
            <th>Delete</th>
        </tr>
    </table>
</div>

Answer 1

The main point of classes is to be able to create multiple instances independently from each other. So if you had the same "widget" multiple times on your page, they could work without needing to write duplicate code. The main problem is that you have things like IDs and URLs hard-coded into your class, and have relevant parts (namely the event handlers) outside of the class.

So, yes, you are missing the point.

If you want to encapsulate everything, you should drop the IDs and use classes (or data- attributes) and hang up everything from a surrounding element, e.g.:

<form class="questioneditor">
   <!-- put everything your widget need in here marked up with classes, not IDs -->
</form>

$(".questioneditor").each(function() {
   new Customerformquestionmanager(this);
});

---

There are several other (small) problems with your code that could be optimized. Here just a quick overview:

• If you have your JSON server-side script return the correct mime-type "application/json" or at least set the option dataType: "json" in the ajax() call, then you don't need to parse the server response yourself. jQuery ajax() will do that for your.
• <br> is for line breaks in things like poems and addresses, not for layout.
• The way you generate the table rows (which is duplicated code BTW) and read the data from them into the input fields looks very dangerous. At the very worst you are opening yourself up to cross-site scripting attacks. At the very least it will probably lead to to headaches with (un)encoding HTML properly.

---

EDIT: Infos to cross-site scripting:

If your data contains any kind of HTML (especially including scripts), then just appending it like this '<td class="rowQuestion">' + data[i].question + '</td>' it won't be escaped and a forgotten <, or a stray </table> will break your layout. And if somehow an infinite loop such as <script>while(1){alert();}</script> get's in, your site will be unusable. At the worst AJAX script could be smuggled in and send personal data to a third party site.

Instead make sure to escape the data by creating proper text nodes. With jQuery, for example, by using the .text() method:

$("<td>").addClass("rowQuestion").text(data[i].question);