Total restructuring of the vicidial.php file

[Accipiter]

I am the lead developer of a Swedish company who hosts dialer systems. We've decided to move towards Open Source, and ViciDial has become our choice.

So, first of all, thanks for the nice project!

However, starting with the task of changing the layout for the agent gui (aka. vicidial.php), I am struck by the major flaws in the code. To mention a few:

* Logic and GUI (HTML) is not separated. Neither in PHP nor Javascript.
* Absolute positioned layout dependant on server-side calculations
* Abuse of Deprecated ereg-expressions.
* Repetitive coding making it difficult to read, modify and extend

My open question is what kind of interest there is for a complete restructuring of the vicidial.php file?

My plan for starters is to:
1) Implement a view class to separate GUI from logic.
2) Separate javascript functions into two groups: Dialer functions (AJAX request performers) and GUI modifiers
3) Modify layout not to be dependant on server-side width/height calculations.

By doing this, creating a custom gui template will be much easier.
I'd prefer not to make my own fork, that is why I am raising the question.

/Samuel[/list]

[mflorell]

What you are describing has been suggested several times before, but no one has been willing to actually put the work, or money, into doing it.

Making the agent interface easy to template is quite a time-consuming task and not a high priority of ours because it really doesn't add any value to our user base.

The other issue is that we are making changes to the vicidial.php script rather frequently, making keeping up with the current codebase more difficult for someone going through all of the work of rewriting the GUI.

We do know about the ereg deprication, and we plan on replacing the thousands of those that we use with preg before 2.4 branches off from trunk.

We did spend quite a bit of time making the vicidial.php script itself XHTML-compliant, and thanks in large part to an outside contribution of help on that project.

We would welcome someone going through the work of doing what you describe, but it should be done in steps that we can integrate as each part is done if possible.

There are a lot of problems with trying to do dynamic(stretchy) spans and divs with the agent interface still keeping all functionality visible and in tact, I would like to know what suggestions you have to solve those issues.

[Michael_N]

You wish to change the layout so that your vicidial looks like your closedsource dialer, so that your clients "feels like home" ?

[ANisus]

(My forum account became disabled after changing e-mail address. I reregistred, but I am still the OP)

@Michael_N:
Yes, partly. A nice, intuitive and welcoming layout is a way to get the agents to more easily adapt to the new system. But it is also about making it more easy to make changes to the code base, add new features or to update the code (take the XHTML update as an example.)

@MattFlorell
Yes, I surely understand the issue about the frequent updates. This would be the main obstacle to overcome in my opinion, to break the project down into smaller steps that wouldn't disrupt other contributions being made.
I am at the moment only at the stage of analyzing the code, categorizing variables and so on.

When it comes to the server side calculations made for the layout, I think this can be solved rather easily.
If one does not wish to change the absolute positioned layout, my suggestion would be this:
Since most widths/heights are style attributes, one could instead set them as attributes to a class. Eg:

Code: Select all

.MNwidth {width:500px}

Then the width will only need to be set once, causing all html-tags with this class to take the width. After this, a new Javascript function could be created that runs onLoad and onResize, checking the current width/height of the browser window, calculating and setting the new width of each class.
By doing this, no width/height values needs to be calculated on the server side, and as a big bonus you may also resize the window, having the layout adapt on the fly.

But anyhow..

Yes, we are willing to put time and effort into restructuring the vicidial.php code. We want to contribute to the project if possible, but since the changes we need to make are quite major (we also need to implement some new non-existing features as well) I am still not sure how to do this without it turning into a fork.
What would be a good way to proceed?

[Michael_N]

http://www.eflo.net/VICIDIALmantis


Here you can put your changes and add of features to vicidial

[mflorell]

The best way to proceed is to work off of the most recent vicidial.php in svn/trunk and post a standard diff file of your changes, then post here on this ticket so we can work to test the changes as soon as possible to we can integrate it into the codebase quickly. This is how we have done similar projects in the past and it has worked well this way.

Of course you should figure out the phases of your project first, because doing everything in one step makes it much more difficult overall.

[ANisus]

I think I am finished with my preliminary analysis.

We have decided to completely rewrite the Agent GUI due to some important factors. This mean, currently, that I will replace vicidial.php and vdc_db_query.php completely.
Some of the main reasons are:

1) Restructuring existing code seemed close to impossible
2) Without restructuring, new features and layout will be difficult to add
3) Existing solution has fatal security flaws, opening up for SQL-injects
4) Many of the existing features are not required for our setup

So, we will rebuild, and step by step add and restructure the features from the old code.

But, we still wish to aid in the project somehow! I really like the idea of OpenSource. So, might there be of any interest to share our version of the GUI? Maybe it can be used in some way?

Also, about the security flaws mentioned in number 3)
In vdc_db_query.php not all variables are escaped before used in SQL-queries.
I found $session_name and $server_ip on row 663 (version 2.4-188, build 110430-1925), but I assume there might be more similar bugs.

[mflorell]

Please post any specific security issues that you have found to the Issue Tracker and post the link back here.

We would love to add your new GUI to the codebase if you would be willing once it is finished.

[williamconley]

The beautiful thing about contributing code like that would be that it is maintained by the Project at that point. New items are maintained within it ... instead of having to re-merge everything new each time a new release comes out OR just "not upgrade".

[mflorell]

I looked into the session_name and server_ip potential SQL injection attack vulnerabilities you mentioned, this is really only an issue if the attacker has an active username and password on the system already, since those are validated before that SQL is even run. I wouldn't call that a fatal security issue, but it is certainly one which we will address before the next official release.

[ANisus]

True Matt, it might not be considered fatal due the agent login required.
If I find any other issues I will post them on the tracker and here.

Hopefully I will have something to show you all soon
Thanks for the input!

@williamconley: Yes, that is our desire too. I just didn't know if a new GUI was of interest. But if it is, then it would be nice letting it be part of the Project.

[williamconley]

In fact a new "Visually pleasing" GUI would likely make you a very popular guy.

[ANisus]

Hehe, yeah, I am aiming for popularity

Naah, but I at first I think people will like it, saying: Ooh, this looks nice
Then they will realize not all features are implemented: Heey! Why doesn't X work?
And finally the developers will check the code and go: Nice, it won't be that hard to reimplement the missing stuff.

All I need is my boss' permission to submit it to the project. But I think he will be easy to convince.

[williamconley]

The convincing moment: If we submit it soon enough and they like it, it becomes part of the project and WE don't have to maintain it ... it updates itself. Saves money. (IF you "get it close enough" to fully functional)

[ANisus]

By the way. Is it a problem if I utilize jQuery in the GUI?
I don't know much about licenses.

And also, I added a trivial bug to the tracker: id=500
(I wasn't allowed to post the link)

[mflorell]

Using jQuery should be fine, although we had many problems with it trying to do complex AJAX queries and AJAX within loops when we tried to use it in the past.

As for your bug, I replied to it, I see that variable set above where you say it is not set so I'm not sure where the problem would be.

[ANisus]

Good about being able to use jQuery.
I hope I can avoid running into problems with it, but time will tell.

Now I have another question that I am a bit stuck on:

I am trying to reduce the information sent from the client to the server.
Reason: To avoid having any hacker-wanna-be agent sending invalid extensions or whatnot as arguments to the server.

But, what information do I actually need to send apart from:
User validation info: $user and $pass
Session validation: $server_ip and $session_name
Agent events: Basically $ACTION, but other things too
Agent submitted data: Could be lead info, fallout, comments and whatnot.

At the moment, a lot is passed to vdc_query_db.php, but what is required?

I do understand that such a change might cause slightly more load towards the database as I need to fetch the info from there instead, but I think it is worth it.

[mflorell]

You would really need to break this down into a function-by-function basis. Most functions are fairly stripped-down as far as what is sent, but there are some variables that were added and later they were made redundant on the vdc_db_query.php side.

[dspaan]

Ansius, have you considered spicing up the reports section? The agent screens can be improved but imho the reports could maybe even be more important and have more impact on the community.

Remember that managers are the decision makers when it comes to using VICIdial or not. And the reports hurt my eyes (sorry Matt, ).

The font is so small and those dotted lines used as borders in some reports are just not of this day and age, they look like something of the 1970's lol.

[saidmsl]

my 2 cents :

just checked osdial code. they already broke the php in smaller files.

Rgds

[mflorell]

Keep in mind that OSDial is based upon ViciDial from 4 years ago. A lot has changed with ViciDial since then.

[williamconley]

And they likely broke the php into smaller files merely for ease of editing.

I could see breaking it into functions and then removing the functions to a functions.php file (which is apparently underway) and removing some java and putting it into .js (which I would not be surprised to see shortly) and then it may be easier to enforce the standards already in use for functions in both languages.

[mflorell]

I'm sure it's possible, it would just take a lot of time, and even more testing, and at the end, there isn't much benefit for the end users, which is why nobody has done it or paid to have it done yet.

[covarrubiasgg]

Mr mflorell i have read you a lot of times (really a lot) saying that improve the coding style does not give much benefit for end users, but i think you are missing a big benefit for end users.

If the code is clean and standards firendly, more people can add features to vicidial and help it grow much faster and solid. Is like and investment at the begining it will look indeed like not much benefit, but in the long run this is what will give the end user the greatest benefit with a bigger comunity of developers.

I really respect your work and all the comunity of developers who are making vicidial the best free call centers suite

[williamconley]

But the point is that it costs money. I don't think Matt is trying to say it's not worth doing, but merely attempting to explain why noone is paying to have it done.

I cannot personally think of a single call center that would be willing to put a couple (or a few) thousand dollars into "cleaning the code" for the possible future benefit of the Project.

There are some very cool call centers will to pay to spice things up (like reports, etc), but still: for immediate benefit (although some of the items may ALSO have long term benefits ...).

[ANisus]

Yes, I do understand Matts point of view.
Cleaning up the code is time consuming, and the result will most likely be a decrease in features for starters.

But, I have started the work, and my company is willing to pay for the development. Vicidial is a great platform, and we are happy to build upon it.

I have also gotten a green light to share our code to the community, but when I requested to post my work so far, they asked me to wait.

But I can just mention where I am at so far:
* Login and presentation is separated. The new "vicidial.php" file uses a view class, putting all the HTML in separate .phtml files.
* Stand alone Javascript files. To use the cache, javascript is moved out of vicidial.php. The javascript is also separated into two main files, one for the base-code and one for layout-code. The base-code takes care of all the communication with the Server and internal logic while the layout-code (using callback function) displays it.
* Static javascript files. To avoid passing a lot of variables from the PHP to the HTML and Javascript directly when calling vicidial.php, most settings are fetched after the agent GUI is loaded. The only true variables used to initialize the Javascript is authentification variables such as username/password.
* Server requests splitted into multiple files. All AJAX requests to the server is routed through a single file, much like [/i]vdc_db_query.php[i]. But unlike the huge vdc_db_query.php file, each ACTION has a separate file, loaded on request.
* jQuery and JSON. To ease up Javascript coding as well as transfer of information to and from the server, jQuery is used as well as JSON.
* Simplified AJAX debugging through FirePHP. The new "vdc_db_query.php" takes care of authentification, JSON encoding as well as error handling. FirePHP is implemented to ease up debugging.
* Javascript variables/methods/functions grouped into objects. To avoid cluttering the global scope, objects are used to group variables and methods. No true OOP as every single object acts as a static class/singleton. Actually, no classes are defined, only objects. But doing it like that works well with jQuery.

So... what can you do so far?
At the moment, not much. You can log in with a user/phone, start a manual call and view the lead information in the header/form (Oh, by the way, the lead form is now built in the Javascript instead of PHP-code.)

Today I will be adding the hangup "feature"


Mvh,
Samuel Jirénius

[cristian]

This is fantastic. Thanks for sharing!

[ANisus]

Ansius, have you considered spicing up the reports section?

Yes and no. We already use a Report generator that we built upon PHPExcel. During our tests of Vicidial/GoAutoDial, we built a bridge between the Vicidial Database and our current reporting database, allowing us to fetch reports through our existing framework.

It was necessary as our customers requires more customizable and exportable reports.

But I don't believe this framework of ours is within the scope of the Vicidial project. But it is still a thought to consider.

Mvh,
Samuel Jirénius

[Acidshock]

I have spent A LOT of time on this trying to figure how the hell I am going to clean it up and I must say that I still havent figured it out lol it is so overwhelming to say the least. Also to keep in mind that there are a lot of changes actively happening and you can try to clean it up but you still need to keep up with the changes. I am confident that once I get going I will eventually crack the egg, or crack myself

[mflorell]

I understand completely. I spend most of my time adding things to Vicidial, so it is always a moving target. The vicidial.php page gets changed about twice a week on average as I add new features(the last one was this morning, adding call_id as a variable sent to scripts/webforms/etc...).

If you have any questions as you work on this, feel free to ask.

[williamconley]

So anything that modifies the pages in question would have to be a script that can be run against the code ... so it can keep up with the code.

And any "irregularities" (where the code breaks its own rule/pattern) could be brought to matt's attention to "perfect" so the fixing script can do its job.

Then when you DO catch up, keeping it caught up during "testing" may not be impossible and it may end up in a "release".

Sorta like the translation system.