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

Improvement: addition of a custom ticket prefix #1386

Closed
wants to merge 1 commit into from
Closed

Improvement: addition of a custom ticket prefix #1386

wants to merge 1 commit into from

Conversation

mikenowak
Copy link
Contributor

Currently the RT# ticket prefix is hardcoded, so these of us who would like to use a custom one are out of luck. This PR adds support for specifying custom ticket prefix via rt.ticket_prefix.

For completeness, I want to clarify that that the users willing to take a full advantagage of this would also need to adjust helpdesk_notification_mail_subject accordingly.

@mikenowak
Copy link
Contributor Author

Sorry, a copy & paste fail - basically ConfigHelper::checkValue() returns true, or false, not the string. I will be fixing this in a moment.

@mikenowak
Copy link
Contributor Author

@chilek, this has been fixed.

@chilek
Copy link
Owner

chilek commented Sep 14, 2018

@miknowak: RT# is not harcoded from some time. You can always use configuration variable settings to format notifications and thus adjusting them to your requirements/needs.
So please try to use following settings:

  • phpui.helpdesk_customerinfo_mail_body
  • phpui.helpdesk_customerinfo_sms_body
  • phpui.helpdesk_notification_mail_subject
  • phpui.helpdesk_notification_mail_body
  • phpui.helpdesk_notification_sms_body

%tid could be converted to %[sprintf_like_symbols]tid as for example:
%06tid would replace this symbol for ticket 345 with 000345.

Any way thanks for contribution - it is seldom seen for LMS project ;-)

@mikenowak
Copy link
Contributor Author

@chilek, I understand what these vars are for, don't get me wrong and its not just the notifications that I am concerned about.

My primary concern is that the following line has an RT# hardcoded, so if my phpui.helpdesk_notification_mail_subject looks like [ISP#%tid] %subject, then unfortunately rtparser expects the ticket in the RT# format, so each response generates a new ticket, rather than linking the response to the already existing ticket. The offending line is this:

if (!$prev_tid && preg_match('/RT#(?<ticketid>[0-9]{6,})/', $mh_subject, $matches)) {

Hope this makes sense now, and puts this PR into perspective for you.

Sure, no problem re the contributions :)

@chilek
Copy link
Owner

chilek commented Sep 14, 2018

@mikenowak I seen now! Let's change it to phpui.helpdesk_ticket_prefix with %tid substitution symbol value. It'll allow to customize format for user. For example:
RT#%06tid for ticket 345 will convert to RT#000345
Wouln't it be very flexible?

@chilek
Copy link
Owner

chilek commented Sep 14, 2018

Perhaps we should go further and allow to format whole subject this way:
RT#%06tid %subject
?

@mikenowak
Copy link
Contributor Author

mikenowak commented Sep 19, 2018

@chilek, ok so as discussed in #1380, I will be implementing this as phpui.helpdesk_notification_ticketid_format.

I will stick to the ticketid format only, and ignore the %subject just as you've described in the below comment sourced from #1380.

So we would have 2-level %tid substitution:
phpui.helpdesk_notification_ticketid_format (replaces [RT#%06tid] by [RT#000345]).
phpui.helpdesk_notification_subject (replaces %tid %subject by [RT#000345] some ticket subject).

@mikenowak
Copy link
Contributor Author

I lost focus on why we were making these changes, so I dediced to go back and look at this issue again.

I found that the real reason why the email replies are not linked to existing tickets is becasue both of the below checks in lms-rtparser.php fail.

$lastref = array_pop($reftab);
...
// check 'References'
if ($lastref) {
	$message = $DB->GetRow("SELECT id, ticketid FROM rtmessages WHERE messageid = ?",
		array($lastref));
	if (!empty($message)) {
		$prev_tid = $message['ticketid'];
		$inreplytoid = $message['id'];
	}
}

On this one it turns out that the References header in the original email from LMS has a value of Array.

References: Array

Next, which is what has been already discussed here.

// check email subject
if (!$prev_tid && preg_match('/RT#(?<ticketid>[0-9]{6,})/', $mh_subject, $matches)) {

This fails because my rt.autoreply_subject has a different prefix than the hardoded RT#.

@chilek
Copy link
Owner

chilek commented Oct 1, 2018

On this one it turns out that the References header in the original email from LMS has a value of Array.

Which original post do you mean?
I've checked it out before the moment and references header was ok, I mean the following scenario:

  1. Customer sends post to helpdesk queue email.
  2. Ticket is created in LMS.
  3. User replies to ticket from previous point.
  4. Customer gets reply via email with valid references header (not Array).

Will you describe your test scenario?

* phpui.helpdesk_message_keep_original_title - controls whether the replies to helpdesk messages should always use the subject of the original message (1st one received)
* phpui.helpdesk_message_mail_subject - allows to specify a custom message subject in helpdesk (i.e. [ISP#%tid] %title)
@mikenowak
Copy link
Contributor Author

@chilek, @interduo hi, its been a while but I got seriously distracted - the new code should do the trick - let me know if anything needs changing.

@mikenowak
Copy link
Contributor Author

@chilek, can we merge this please?

@interduo
Copy link
Collaborator

interduo commented Apr 3, 2019

Could You also use newly added ticket prefix in PR?

@interduo
Copy link
Collaborator

interduo commented Apr 22, 2021

This is done by now. The code contains rt.subject_ticket_regexp_match.

@interduo interduo closed this Apr 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants