Skip to content
This repository was archived by the owner on Sep 7, 2020. It is now read-only.

Autoresolve for "expired" tickets#139

Open
khalilsarwari wants to merge 4 commits into
masterfrom
khalil/wait
Open

Autoresolve for "expired" tickets#139
khalilsarwari wants to merge 4 commits into
masterfrom
khalil/wait

Conversation

@khalilsarwari
Copy link
Copy Markdown
Contributor

To maximize the accuracy of estimated wait time, this PR will autoresolve students being assisted for longer than 30 minutes, and delete tickets that have been on the queue for more than 3 hours.

Copy link
Copy Markdown
Member

@jathak jathak left a comment

Choose a reason for hiding this comment

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

I've made some comments on how this is structured.

At a higher level, I think changing assigned tickets to show the time since they were assigned (rather than the time since they were created) is a more useful change than this.

Comment thread oh_queue/views.py Outdated
event_type=event_type,
ticket=ticket,
user=current_user,
user_id=0 # assuming ids start @ 1, 0 signifies admin/autodelete
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This seems hacky. I'd prefer to change TicketEvent to allow the user to be nullable (though that would require a migration)

Comment thread oh_queue/models.py Outdated
return first_name

TicketStatus = enum.Enum('TicketStatus', 'pending assigned resolved deleted')
TicketStatus = enum.Enum('TicketStatus', 'pending assigned resolved deleted autoresolved autodeleted')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think a separate "autoresolved" category is necessary. I don't see any situation where we'd need to be able to distinguish resolved vs autoresolved tickets (and if we really need to, we could always inspect the events)

I'd lean toward the same for autodeleted tickets, but I'm more open to a separate category there.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That makes sense, I'll not add any categories here.

Comment thread oh_queue/views.py Outdated
emit_event(ticket, TicketEventType.delete)
db.session.commit()

@socketio.on('autodelete')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should only be triggered on the server-side, so I don't think the socket event listener is necessary.

Comment thread oh_queue/views.py Outdated
db.session.commit()
return get_next_ticket()

@socketio.on('resolve')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

See comment about autodelete above (also, this shouldn't have the same socket event as the existing resolve function.

Comment thread manage.py Outdated
question=random.randrange(1, 6),
location=random.choice(['109 Morgan', '247 Cory']),
)
if i % 2 == 0:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think this is really necessary. If we do want to keep this, you should create the ticket outside the if and then update the fields for certain values.

@khalilsarwari khalilsarwari changed the title Autoresolve/delete for "expired" tickets Autoresolve for "expired" events Oct 8, 2017
@khalilsarwari khalilsarwari changed the title Autoresolve for "expired" events Autoresolve for "expired" tickets Oct 8, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants