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

Feature/Time Tracking #5

Merged
merged 11 commits into from Jun 23, 2016
Merged

Feature/Time Tracking #5

merged 11 commits into from Jun 23, 2016

Conversation

robintoy
Copy link

@robintoy robintoy commented Jun 22, 2016

Feature / Time Tracking

First release / update going from v1.9.14 to v1.10-rc2

To Do List

  • Rebase pull to develop-next (v.1.11.x) as required for feature submission
  • Remove the current time spend display from post reply and internal note to clean up display
  • Remove total from hardware table and make this calculated
  • Remove Add time Tab
  • Remove total time from ticket table and make this amount calculated on ticket load
  • Clean up / remove hardware and place into a new branch
  • Clean up comments and other mess ready for commit

Files modified

include/class.config.php
include/class.nav.php
include/class.thread.php
include/class.ticket.php
include/client/view.inc.php
include/i18n/en_US/help/tips/settings.tickettime.yaml
include/i18n/en_US/list.yaml
include/staff/org-view.inc.php
include/staff/settings-tickettime.inc.php
include/staff/templates/thread-entry.tmpl.php
include/staff/templates/ticket-print.tmpl.php
include/staff/ticket-view.inc.php
scp/org_bill.php
scp/settings.php
scp/tickets.php
scp/tickets_bill.php
scp/tickets_cost.php

Database Modifications

Run the following commands in something like PHPMyAdmin or MySQL CLI

INSERT INTO  ost_list  ( name ,  name_plural ,  sort_mode ,  masks ,  type ,  notes ,  created ,  updated ) VALUES ('Time Type', 'Time Types', 'SortCol', '13', 'time-type', 'Time Spent plugin list, do not modify', NOW(), NOW());
INSERT INTO `ost_list_items` (`list_id`, `status`, `value`, `sort`)
SELECT ost_list.id, 1, 'Telephone', 1
FROM ost_list
WHERE `name`='Time Type';

INSERT INTO `ost_list_items` (`list_id`, `status`, `value`, `sort`)
SELECT ost_list.id, 1, 'Email', 2
FROM ost_list
WHERE `name`='Time Type';

INSERT INTO `ost_list_items` (`list_id`, `status`, `value`, `sort`)
SELECT ost_list.id, 1, 'Remote', 3
FROM ost_list
WHERE `name`='Time Type';

INSERT INTO `ost_list_items` (`list_id`, `status`, `value`, `sort`)
SELECT ost_list.id, 1, 'Workshop', 4
FROM ost_list
WHERE `name`='Time Type';

INSERT INTO `ost_list_items` (`list_id`, `status`, `value`, `sort`)
SELECT ost_list.id, 1, 'Onsite', 5
FROM ost_list
WHERE `name`='Time Type';
INSERT INTO  ost_config  (`namespace`, `key`, `value`, `updated`) VALUES
 ('core', 'isclienttime', 0, now()),
 ('core', 'isthreadtime', 0, now()),
 ('core', 'isthreadtimer', 0, now()),
 ('core', 'isthreadbill', 0, now()),
 ('core', 'isthreadbilldefault', 0, now()),
 ('core', 'istickethardware', 0, now());
ALTER TABLE  ost_thread_entry  ADD COLUMN  time_spent  INT( 11 ) UNSIGNED NOT NULL DEFAULT '0' AFTER  type;

ALTER TABLE  ost_thread_entry  ADD COLUMN  time_type  INT( 11 ) UNSIGNED NOT NULL DEFAULT '0' AFTER  time_spent;

ALTER TABLE  ost_thread_entry  ADD COLUMN  time_bill  INT( 11 ) UNSIGNED NOT NULL DEFAULT '0' AFTER  time_type;

First release / update going from v1.9.14 to v1.10-rc2
@greezybacon
Copy link

@robintoy you need to change your text editor to use four spaces instead of a tab

@robintoy
Copy link
Author

@greezybacon which files are affected??
I'll look at Notepad++ and see what I can do

@robintoy
Copy link
Author

Going to sound odd by why four spaces instead of tab anyway?

@robintoy
Copy link
Author

Just doing some testing and first error I get is

Fatal error: Call to a member function isThreadTime() on null in /home/hp3-linc1-nfs2-y/538/531538/user/htdocs/timetracking/include/staff/templates/thread-entry.tmpl.php on line 67

Just tracking now what I have missed as the DB is updated and this feature is enabled :( I'll get there!

@robintoy
Copy link
Author

Ok,
Not sure if I'm nuts here but the code for this is located in /include/class.config.php on line 242 and reads

function isThreadTime() {
        // determines if Ticket Time via Threads is Enabled
        return ($this->get('isthreadtime'));
    }

This code looks perfect as it returns either a 0 or 1 depending if this is enabled or not, which works in the settings menu to turn this on.

Any clues @greezybacon ??

@kest874
Copy link

kest874 commented Jun 22, 2016

image

Settings in notepad + to change

@robintoy
Copy link
Author

Thanks @kest874
found that, just did not understand why the need

@greezybacon
Copy link

@robintoy the rest of the code uses four spaces, and tabs are commonly expanded to eight, so it looks inconsistent when viewing. Some files, like YAML expect spaces over tabs.

This should fix your crash

diff --git a/include/class.thread.php b/include/class.thread.php
index 199627e..6fbe786 100644
--- a/include/class.thread.php
+++ b/include/class.thread.php
@@ -233,6 +233,7 @@ class Thread extends VerySimpleModel {

     // Render thread
     function render($type=false, $options=array()) {
+        global $cfg;

         $mode = $options['mode'] ?: self::MODE_STAFF;

You do not have to add 'customization' comments to your code. Those would be removed when merged into the main project, and git keeps track of your changes. So you don't have to explicitly call them out in the code. Instead, write a line or two as to why your code is necessary, if an explanation would add context when reading the code.

@robintoy
Copy link
Author

I understand what you mean, at the moment I have added comments everywhere to help me learn the new layout and get back to areas quickly; but I shall remove before this is sent as a pull to osTicket/develop-next

@robintoy
Copy link
Author

That code did help and seems you run a quick git command to see what was different??

I have been using this link
osTicket/osTicket@develop-next...greezybacon:feature/time-tracking-11

But I now have an issue with the ORM class, but I cannot see you did any changes to that?

Fatal error: Uncaught exception 'OrmException' with message 'ThreadEntry: time_spent: Field not defined' in /home/hp3-linc1-nfs2-y/538/531538/user/htdocs/timetracking/include/class.orm.php:311 Stack trace: #0 /home/hp3-linc1-nfs2-y/538/531538/user/htdocs/timetracking/include/class.orm.php(631): VerySimpleModel->get('time_spent', NULL) #1 /home/hp3-linc1-nfs2-y/538/531538/user/htdocs/timetracking/include/class.orm.php(626): AnnotatedModel->get('time_spent') #2 /home/hp3-linc1-nfs2-y/538/531538/user/htdocs/timetracking/include/staff/templates/thread-entry.tmpl.php(68): AnnotatedModel->__get('time_spent') #3 /home/hp3-linc1-nfs2-y/538/531538/user/htdocs/timetracking/include/staff/templates/thread-entries.tmpl.php(35): include('/home/hp3-linc1...') #4 /home/hp3-linc1-nfs2-y/538/531538/user/htdocs/timetracking/include/class.thread.php(255): include('/home/hp3-linc1...') #5 /home/hp3-linc1-nfs2-y/538/531538/user/htdocs/timetracking/include/staff/ticket-view.inc.php(502): Thread->render(Array, Array) #6 /home/hp3-linc1-nfs2-y/53 in /home/hp3-linc1-nfs2-y/538/531538/user/htdocs/timetracking/include/class.orm.php on line 311

@greezybacon
Copy link

@robintoy you probably need to restart Apache / php-fpm

@robintoy
Copy link
Author

Sorry to be a pain @greezybacon
I have broken the error into chucks to read it better and it looks like it is all pointing to the field "time_spent" not existing??

Fatal error: Uncaught exception 'OrmException' with message 'ThreadEntry: time_spent: Field not defined'
in /home/hp3-linc1-nfs2-y/538/531538/user/htdocs/timetracking/include/class.orm.php:311

Stack trace:
#0 /home/hp3-linc1-nfs2-y/538/531538/user/htdocs/timetracking/include/class.orm.php(631): VerySimpleModel->get('time_spent', NULL)
#1 /home/hp3-linc1-nfs2-y/538/531538/user/htdocs/timetracking/include/class.orm.php(626): AnnotatedModel->get('time_spent')
#2 /home/hp3-linc1-nfs2-y/538/531538/user/htdocs/timetracking/include/staff/templates/thread-entry.tmpl.php(68): AnnotatedModel->__get('time_spent')
#3 /home/hp3-linc1-nfs2-y/538/531538/user/htdocs/timetracking/include/staff/templates/thread-entries.tmpl.php(35): include('/home/hp3-linc1...')
#4 /home/hp3-linc1-nfs2-y/538/531538/user/htdocs/timetracking/include/class.thread.php(255): include('/home/hp3-linc1...')
#5 /home/hp3-linc1-nfs2-y/538/531538/user/htdocs/timetracking/include/staff/ticket-view.inc.php(502): Thread->render(Array, Array)
#6 /home/hp3-linc1-nfs2-y/53 in /home/hp3-linc1-nfs2-y/538/531538/user/htdocs/timetracking/include/class.orm.php on line 311

@greezybacon
Copy link

@robintoy did you restart apache / php-fpm/ php dev server / whatever you are using to serve osTicket?

@robintoy
Copy link
Author

Wish I could, damn hosted system which you cannot restart.

@greezybacon
Copy link

Change the SECRET_SALT in your config file. You might need to reset the passwords for email addresses after you do that, if you have any.

@robintoy
Copy link
Author

does that include admin logins etc?

@greezybacon
Copy link

No. Just email passwords

@robintoy
Copy link
Author

Salt not help I'm afraid

@greezybacon
Copy link

Then it must really not exist in your database?

@robintoy
Copy link
Author

I wish that was true, but looking at my DB in the ost_ticket table I see it
dbexport

@greezybacon
Copy link

@robintoy you should be looking at the thread_entry table

@robintoy
Copy link
Author

You are so right, I feel such a plank!
I'm guessing my old DB had the fields removed when I upgraded to osTicket v1.10

Running lots of SQL now!

@robintoy
Copy link
Author

Spotted it!!!
the old table in v1.9.x was ost_ticket_thread which is now replaced with ost_thread_entry

@robintoy
Copy link
Author

Worked like a charm!!!
Thank you @greezybacon

@greezybacon
Copy link

No worries. Glad your back on track

@robintoy
Copy link
Author

I'll get there, just lots have changed from v1.9.x so have been a little lost and out of water; but hopefully I can get this working great and to a level you like as much as the users.

@greezybacon
Copy link

@robintoy perhaps you'll understand why it's been a slow process to release a "stable" version

@robintoy
Copy link
Author

@greezybacon I try not to bug and cause issues :)
We have been using v1.10-rc2 almost since release in production and very happy with it; personally I cannot stand jumping back as it very clunky

Added global var for cfg to settings could be checked
Cleaned up the post reply and post internal note by removing the
"Current Time Spent" information on the forms as this is not needed and
looks out of place.
Removed the Add Time Tab as this does not currently have a path to the
way this mod and osTicket want this to go.
@robintoy
Copy link
Author

Hi @greezybacon
I'm rushing through the changes to get this fully working and just have a question about one of your comments I made into a task.

"Look at changing it from time to expense"

What exactly do you mean with this?

Remove total time from ticket table and make this amount calculated on
ticket load
@greezybacon
Copy link

I think i meant "Look at changing it from hardware to expense". Not everybody is in the IT industry, but most people interested in this feature would like to track some expenses counted toward a ticket.

@robintoy
Copy link
Author

That makes sense @greezybacon

I'm thinking about cleaning up / removing hardware for a moment as that would mean this is complete for time (I believe) and ready for a pull to your system?

Then I was thinking of making a couple more branches: -

  • Expenses
  • Contracts

where I can work on these sections independently of the time section, I think this sticks more to the way you want to add code and allows us to work towards getting the base time part in the main release.

What you think?

Removed the hardware sections of the code as this will be made in to
another branch and renamed to "expense" so not so IT related.
@greezybacon
Copy link

@robintoy I don't have an immediate problem with that approach. The problem is, this software is downloaded several hundred times a day, and everybody who uses it has a different need for the software. The problem with new features, is that while they have a great following, once they make their way into the software, they will have an even greater following. Then, folks will complain that it doesn't work the way it "obviously should". And that's where things get complicated.

If we add the time tracking by itself, it needs to be quite polished. And it needs to apply to almost any angle someone might want to use it.

So if that's the direction you want to go, finish up your proposal, and I'll review it more specifically.

@robintoy
Copy link
Author

robintoy commented Jun 23, 2016

@greezybacon I was thinking if I get time working fully, just thinking I might look at added an Invoiced field so billed thread information can be marked as invoiced and not appear in later reports then we have hopefully a rather polished system. (This really should be part of contracts and SLAs)

Expenses would then be a great / nice improvement, but as a tab instead of a new window so it is in keeping with your system, and not a hack.

At present most people do not use the hardware as far as I;m aware as they do not know its there.

I hope the makes sense and makes improvements better for the system, and might allow us to get all in for rc3/4

I have removed the time export as the field in question is no longer
part of the mod
Added thread time spent and type to PDF print
Cleaned up and removed all older non-used code and comments from the
code keeping it clean ready for pull to osTicket:develop-next
Added back the missing Org Bill Report
Re-Save of settings YAML to remote tabs
@robintoy
Copy link
Author

Hi @greezybacon
I have been through and done a mass testing and clean up of the code etc....

Cannot see any errors and works smoothly, do you think I'm ready to merge this to our develop-next and then create a pull to your develop-next for review and hopeful inclusion?

@robintoy
Copy link
Author

Hi @greezybacon
about to take the plunge and request a pull!

@robintoy robintoy merged commit 8c63161 into develop-next Jun 23, 2016
@kest874
Copy link

kest874 commented Jun 23, 2016

Thought I would check this out, I have it working. Was wondering is there a way to edit time?

If I have the timer running and walk away and forget and then post a reply it will log the accumulated time. If I know it's not accurate can I edit it?

@robintoy
Copy link
Author

Hi @kest874
Very interesting question and the answer is not 100% simple as I have two possible answers: -

  1. Within osTicket = no
  2. When using the system is does have a pause button on the timer which you should push before walking away just like locking your workstation so no one else can access your documents, emails etc.

I take your point and is something that would be a good improvement to this once included in the base?

@kest874
Copy link

kest874 commented Jun 23, 2016

I can see someone miss-entering time on accident. It should require permissions to make an adjustment, and be logged in the thread.

Maybe entertain the ability to put negative minutes in as an alternative.

@robintoy
Copy link
Author

Final pull to main code of v1.10-rc3 has now been submitted
osTicket#3231

@robintoy robintoy deleted the TimeTracking branch August 5, 2021 07:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants