731 multiple support queues #67
Conversation
…move RT env var config BREAKING CHANGE: Configuring the RT client through environment variables is no longer supported.
…al:Logging RT queue
| }) | ||
| }) | ||
|
|
||
| it('Should create support ticket', () => { | ||
| cy.task('log', `Support Ticket creationEnabled=${supportTicket.creationEnabled}`) |
There was a problem hiding this comment.
It would be nice to log the queue name as well.
| @priority = config[:priority] | ||
|
|
||
| if !@queue || !@priority | ||
| raise ArgumentError, "queue_name and priority are required options for RT service" | ||
| if !@queues || !@priority |
There was a problem hiding this comment.
Should be check for empty queues too?
| @@ -30,8 +31,9 @@ def create | |||
| end | |||
|
|
|||
| rts = RequestTrackerService.new ::Configuration.request_tracker_config | |||
| rts.selected_queue = queue | |||
There was a problem hiding this comment.
Now that the queue name can be overriden with a request parameter, should the support_ticket object carry this value? The overriden value can be set into support_ticket in create_support_ticket function.
There was a problem hiding this comment.
I had initially thought that the queue selection would make sense as part of the SupportTicket object, but I moved it to the RequestTrackerService object when I saw that only the latter has the information from the client configuration file, which is necessary in order to validate the queue selection. This way, the queue selection can be validated at the time of assignment. If the queue selection is part of the SupportTicket object, then it cannot be validated until the SupportTicket object is passed to the RequestTrackerService.create_ticket method.
Do you think that it is ok to wait to validate the queue selection until this late in the execution flow?
There was a problem hiding this comment.
I see. For me it is ok to do input validation in the create_ticket method.
The only thing I am wary of is adding selected_queue as a class field. This makes the service a transient object (although this is the way we are using it anyway)
There was a problem hiding this comment.
Ok. I was concerned it might be a problem if the SupportTicket object validates some of its instance variables at assignment and others are not validated until their use, but if you are ok with this then I will move the variable there.
| @@ -14,5 +14,6 @@ | |||
| {"token": "Stata", "name": "Stata"}, | |||
| {"token": "SAS", "name": "SAS"}, | |||
| {"token": "Matlab", "name": "Matlab"} | |||
| ] | |||
| ], | |||
| "support_ticket": {"creationEnabled": true, "queue": "Internal:Logging"} | |||
There was a problem hiding this comment.
support_ticket default values can be set into https://github.com/hmdc/sid2/blob/canary/automated-testing/cypress/support/base.js#L33 for simplicity
There was a problem hiding this comment.
Yes, but since these are site-specific values, I prefer that they go in configuration files.
| @@ -15,5 +15,5 @@ | |||
| {"token": "SAS", "name": "SAS"}, | |||
| {"token": "Matlab", "name": "Matlab"} | |||
| ], | |||
| "support_ticket": {"creationEnabled": false} | |||
| "support_ticket": {"creationEnabled": true, "queue": "Internal:Logging"} | |||
There was a problem hiding this comment.
creation enabled in remote-dev will require the production rt_config file to be added manually to the dashboard folder before deploying.
| user: "root" | ||
| pass: "password" | ||
| queues: | ||
| - "General" |
There was a problem hiding this comment.
Do we need to add the "Alternate" queue in the local rt_config.yml?
There was a problem hiding this comment.
No, the RT client configuration for local dev only needs to specify one queue in order to be functional. I only included an additional (nonexistent) queue in the RT client configuration for automated-testing so that the test suite could test what happens when a configuration file is loaded that contains multiple configured queues.
…o SupportTicket class
…g + removing unused variable
No description provided.