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

tickets views #1158

Open
wants to merge 15 commits into
base: develop
Choose a base branch
from
Open

tickets views #1158

wants to merge 15 commits into from

Conversation

ssteeltm
Copy link
Contributor

Kanban view
Compact view

Kanban view
Compact view
@ssteeltm ssteeltm marked this pull request as draft January 29, 2025 14:03
@ssteeltm ssteeltm marked this pull request as ready for review January 29, 2025 14:42
sync last changes on days  27 and 28
@ssteeltm ssteeltm marked this pull request as draft January 29, 2025 17:28
@ssteeltm ssteeltm marked this pull request as ready for review January 29, 2025 18:58
@ssteeltm ssteeltm marked this pull request as draft January 29, 2025 19:05
@ssteeltm ssteeltm marked this pull request as ready for review January 29, 2025 19:36
tickets.php Show resolved Hide resolved
tickets.php Outdated

?>
if ($_GET["view"]) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning: Undefined array key "view" in tickets.php on line 358
This too.

Also, for the most part we do PHP logic at the very top of the page, unless it makes good sense to do otherwise

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, indeed. but in this case that tickets.php being modular, I couldnt see a simpler way. In the future we can change this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets keep like this for now. Later we can think a better way to do this?

tickets_kanban.php Outdated Show resolved Hide resolved
tickets_kanban.php Outdated Show resolved Hide resolved
continue;
}

mysqli_query($mysqli, "UPDATE tickets SET ticket_kanban = $kanban, ticket_status = $status WHERE ticket_id = $ticket_id");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved tickets should be setting ticket_resolved_at, and un-resolving a ticket should unset it.

If $config_ticket_client_general_notifications is enabled, we should be sending the contact & any watchers an email to say it's resolved.

Copy link
Contributor Author

@ssteeltm ssteeltm Jan 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Resolved tickets should be setting ticket_resolved_at, and un-resolving a ticket should unset it.
  • If $config_ticket_client_general_notifications is enabled, we should be sending the contact & any watchers an email to say it's resolved.

Copy link
Contributor Author

@ssteeltm ssteeltm Jan 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you sugest to have the notifications? As I see theres no function to do so, its all coded in ticket posts. The way I see now is to duplicate codes in ajax.php

@wrongecho
Copy link
Collaborator

Hey!

This is some really nice work here, well done!! I've left a couple comments for you to look over.

Generally speaking, I think we should exclude 'Closed' being a state on the board. I'm fine with resolved, but our stance with a ticket being closed is that you don't get to re-open it afterwards. Currently, the kanban bypasses the resolved -> closed flow and also lets you re-open tickets.
It probably also makes sense to hide the Kanban option when you are just viewing closed tickets (tickets.php?status=Closed) as you can't do anything with them.

Also, I'm thinking we should remove the ability to move columns around and rename ticket_status_kanban to something like ticket_status_order so it's set in the admin settings (like we do in custom_links). That way the admin determines the flow (New > Open > Hold > Custom > Resolved) rather than any random tech. We can use the order elsewhere too.

Thoughts? :)

@ssteeltm
Copy link
Contributor Author

ssteeltm commented Jan 31, 2025

Hey!

This is some really nice work here, well done!! I've left a couple comments for you to look over.

Generally speaking, I think we should exclude 'Closed' being a state on the board. I'm fine with resolved, but our stance with a ticket being closed is that you don't get to re-open it afterwards. Currently, the kanban bypasses the resolved -> closed flow and also lets you re-open tickets. It probably also makes sense to hide the Kanban option when you are just viewing closed tickets (tickets.php?status=Closed) as you can't do anything with them.

Also, I'm thinking we should remove the ability to move columns around and rename ticket_status_kanban to something like ticket_status_order so it's set in the admin settings (like we do in custom_links). That way the admin determines the flow (New > Open > Hold > Custom > Resolved) rather than any random tech. We can use the order elsewhere too.

Thoughts? :)

I agree with you I did changes now to keep resolved rules that you mentioned, so Resolved will remain on columns
About the moving columns, liked ur ideia, but how about doing permissions on this? so only admin can move this columns

@ssteeltm
Copy link
Contributor Author

ssteeltm commented Jan 31, 2025

"t probably also makes sense to hide the Kanban option when you are just viewing closed tickets (tickets.php?status=Closed) as you can't do anything with them."

About this, makes all sense, will update to behave like that

Done

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