HR: Two scripts to maintain departments

[pakricard]

@timschofield :

When installing the new HR module, the 53.php updates the department table, adding some fields. OK.

The Script HRDepartments.php takes care of the maintenance of the departments table, but we do still have the old Departments.php that maintains the same table, but the new fields are not maintained there.

So, I guess we should remove the old Departments.php script from webERP, and make the old menu item Setup -> Maintain Internal Departments to point as well to the new HRDepartments.php

or, rename the new HRDepartments.php as Departments.php

Anyway, we have now 2 scripts to maintain the same table and one of them is obsolete.

R

[pakricard]

Thinking a bot more about this:

Are the new “HR departments” the same concept as the previous “Internal departments”?

Until now, Internal Departments are only used for Internal Stock Request between departments. Something like “Area 123 requests 1 laptop to IT department”, where Area 123 is one of the 999 shop areas assigned to Sales Department.
But the new HR departments are (as I understand them) something like “John Doe works in Sales Department”

Can we accommodate for this scenario?
R

[pakricard]

Another one: line 52 if 53.php updates the department code:

$SQL = "UPDATE departments SET departmentcode = CONCAT('DEPT', LPAD(departmentid, 4, '0')) WHERE departmentcode IS NULL OR departmentcode = ''";

But later on, HRDepartment.php does not keep this format when updating a Department:

If I update the name of DEPT0003, I get also the code updated (to departmentID)

• So, is line 52 of 53.php really needed?
• Why do we have a departmentid AND a departmentcode?
• Should departmentcode be maintained (user-editable) as well in HRDepartments.php?

R

[timschofield]

At one point in the development of the HR module, HRDepartments.php used a table called hrdepartments and Departments.php used a table called departments. This seemed wrong as it was just duplicating data. I decided to standardise on the departments table and get rid of hrdepartments (moving the fields from hrdepartments to departments), as I try to avoid deleting existing tables in case people use that table in customisations. I was also aware that some people who use the ISR functionality wouldn't want to use the HR functionality, and so would need an independent way of maintaining the department information. On reflection I think I agree that having both links points to HRDepartments.php is the best solution, though there should still be the two menu item links.

[pakricard]

At one point in the development of the HR module, HRDepartments.php used a table called hrdepartments and Departments.php used a table called departments. This seemed wrong as it was just duplicating data. I decided to standardise on the departments table and get rid of hrdepartments (moving the fields from hrdepartments to departments), as I try to avoid deleting existing tables in case people use that table in customisations. I was also aware that some people who use the ISR functionality wouldn't want to use the HR functionality, and so would need an independent way of maintaining the department information. On reflection I think I agree that having both links points to HRDepartments.php is the best solution, though there should still be the two menu item links.

OK. Makes sense to me.

Then the list of issues to fix regarding this "2 scripts for 1 table" is:

1. Point "Setup -> Maintain Internal Departments" to the new HRDepartments.php
2. Delete Departments.php

Regarding departmentcode:

1. For initial setup: I think updates/53.php line 52
   instead of
   $SQL = "UPDATE departments SET departmentcode = CONCAT('DEPT', LPAD(departmentid, 4, '0')) WHERE departmentcode IS NULL OR departmentcode = ''";
   should be
   $SQL = "UPDATE departments SET departmentcode = departmentid;

2. departmentcode should be editable in HRDepartments.php

Some more details about HRDepartments.php

1. Authoriser drop-down should be sorted by full name, not username (or showing the username first)

R

[timschofield]

1. Point "Setup -> Maintain Internal Departments" to the new HRDepartments.php

2. Delete Departments.php

Agreed, I will do that later.

Regarding departmentcode:

1. For initial setup: I think updates/53.php line 52
   instead of
   `$SQL = "UPDATE departments SET departmentcode = CONCAT('DEPT', LPAD(departmentid, 4, '0')) WHERE departmentcode IS NULL OR departmentcode = ''";`
   should be
   `$SQL = "UPDATE departments SET departmentcode = departmentid;`

When I was creating it, I envisaged that the department code would be descriptive, and the department id is an auto incremented integer. The problem was what to do with existing departments. I wanted to emphsise that ID was different to code, but I didn't know how to programatically create a descriptive code. This above was my compromise.

2. departmentcode should be editable in HRDepartments.php

Tricky let me think more about it

Some more details about HRDepartments.php

1. Authoriser drop-down should be sorted by full name, not username (or showing the username first)

Currently it is sorting it by last name, and then first name if last name is the same.

R

[pakricard]

Hi @timschofield:

When I was creating it, I envisaged that the department code would be descriptive, and the department id is an auto incremented integer. The problem was what to do with existing departments. I wanted to emphsise that ID was different to code, but I didn't know how to programatically create a descriptive code. This above was my compromise.

Then, we will have 2 unique identifiers in that table: id and code. id for internal department scripts and code for HR ones. It is OK as we don't mix it later in the future. Probably the best course f action should be getting rid of id in favour of code, modifying the internal requests scripts. But it is quite a task.

Then, if we keep both (id and code), the code should be created and edited correctly.
Let's use an "imported from old departments table" department. it will have the DEPT0038 as code (if the id was 38). OK until here.
Try to edit it:

The code shown is, in fact, the id. Not the code.
Edit the name of the department (or any other field) and the department code will turn into 38. This is a bug as it changes the code even if the script doesn't allow the user to.

Currently it is sorting it by last name, and then first name if last name is the same.

I think we talk about different things, then. Checked HRDepartments.php, line 304 and it does populate from www_users:

// Authoriser dropdown
echo '<field>
		<label for="Authoriser">' . __('Authoriser') . ':</label>
		<select name="Authoriser">';
echo '<option value="">' . __('None') . '</option>';
$SQL = "SELECT userid, realname FROM www_users ORDER BY userid";
$Result = DB_query($SQL);
while ($MyRow = DB_fetch_array($Result)) {
	if (isset($_POST['Authoriser']) AND $_POST['Authoriser'] == $MyRow['userid']) {
		echo '<option selected="selected" value="' . $MyRow['userid'] . '">' . $MyRow['realname'] . '</option>';
	} else {
		echo '<option value="' . $MyRow['userid'] . '">' . $MyRow['realname'] . '</option>';
	}
}
echo '</select>
	</field>';

if using www_users should be sorted by realname, if not showing the username.

If you prefer to use table hremployees, then maybe we need to import current data from www_users but there is probably not a 1:1 relationship: www_users could have non-employees (customers or suppliers). And www_users has "realname", which will be difficult to proper separate into first and last name.

R

[dalers]

try to avoid deleting existing tables in case people use that table in customisations.

Personally I think the first consideration should be the best interests of the project, which to me is functionality of value to users followed by stability, simplicity and maintainability. Maintaining backwards compatibility for users who may may have developed dependent code should be the last concern and it can be mitigated by providing advance notice of breaking changes (such as renaming the manufacturers table and a salescatprod table column).

[timschofield]

Then, we will have 2 unique identifiers in that table: id and code. id for internal department scripts and code for HR ones. It is OK as we don't mix it later in the future. Probably the best course f action should be getting rid of id in favour of code, modifying the internal requests scripts. But it is quite a task.

I'm not sure I see a problem here. The user doesn't need to care about the id, in fact probably best to remove it from the screen completely.

Then, if we keep both (id and code), the code should be created and edited correctly. Let's use an "imported from old departments table" department. it will have the DEPT0038 as code (if the id was 38). OK until here. Try to edit it: The code shown is, in fact, the id. Not the code. Edit the name of the department (or any other field) and the department code will turn into 38. This is a bug as it changes the code even if the script doesn't allow the user to.

Yes, I had already fixed this, but was waiting to resolve the other issues before committing it.

Currently it is sorting it by last name, and then first name if last name is the same.

I think we talk about different things, then. Checked HRDepartments.php, line 304 and it does populate from www_users:

My apologies, I was looking at manager not authoriser.

If you prefer to use table hremployees, then maybe we need to import current data from www_users but there is probably not a 1:1 relationship: www_users could have non-employees (customers or suppliers). And www_users has "realname", which will be difficult to proper separate into first and last name.

I thought a lot about this, but as you say there will be employees without webERP accounts. It would be possible to use hremployees as it has a field for webERP userid, but this maybe a situation where the effort involves is greater than the benefits.

[timschofield]

try to avoid deleting existing tables in case people use that table in customisations.

Personally I think the first consideration should be the best interests of the project, which to me is functionality of value to users followed by stability, simplicity and maintainability. Maintaining backwards compatibility for users who may may have developed dependent code should be the last concern and it can be mitigated by providing advance notice of breaking changes (such as renaming the manufacturers table and a salescatprod table column).

I think you are misunderstanding here. This is a toss of the coin decision on whether we standardise on hrdepartments or departments. It seems to me to be common sense, in a situation such as this, to say that we choose the option that is least likely to cause issues for existing users of webERP. Would you not agree?

Tim

[timschofield]

OK, I have committed my fix for the bug @pakricard mentioned above.

I have also hidden departmentid from the user completely.

I have also made departmentcode editable.

Tim

[dalers]

that we choose the option that is least likely to cause issues for existing users of webERP. Would you not agree?

Hi Tim, my point was only that the possibility of users with affected custom code should not be as high on the consideration list as the concerns of those who are known to use, develop and maintain webERP (e.g. functionality of value followed by stability, simplicity and maintainability).

Imho, users have a responsibility to participate in the community. If there are users who depend on backwards compatibility, is it not reasonable to expect they participate in discussions on breaking changes and make an argument why a breaking change should not be made?

[timschofield]

There are multiple things to take into account when making a major change, such as deleting an existing table. One of those things is the affect of the change on existing users. It would be irresponsible not to consider it. However nobody is suggesting it should be the only criteria.

[pakricard]

I'm not sure I see a problem here. The user doesn't need to care about the id, in fact probably best to remove it from the screen completely.

Agree. In fact, we hide id in many other scripts. "Internal use only".

I thought a lot about this, but as you say there will be employees without webERP accounts. It would be possible to use hremployees as it has a field for webERP userid, but this maybe a situation where the effort involves is greater than the benefits.

Understood.

I commited a couple of minor improvements: "Authoriser" changed to "Internal requests authoriser", and ORDER BY realname.

Another minor issue:
Line 272-275

$SQL = "SELECT employeeid, CONCAT(firstname, ' ', lastname) as fullname
		FROM hremployees
		WHERE employmentstatus = 'Active'
		ORDER BY lastname, firstname";

If order is by last name and first name, should line 272 be
$SQL = "SELECT employeeid, CONCAT(lastname, ', ', firstname) as fullname
so it is shown in the same order used, and visually makes it easier to find the employee you are looking for.

R