buerokratt / Buerokratt-Chatbot

1 stars 18 forks source link

Administrator can turn on/off displaying CSAs name and/or title in End Users chat #10

Open rasmusei opened 2 years ago

rasmusei commented 2 years ago

AS AN Administrator I WANT TO be able to turn on/off displaying CSA's name and/or title in End Users chat SO THAT I can change it according to organization's policy

Acceptance Criteria

Image

janaliiv commented 1 year ago

This feature probably needs new config files for YML-based Ruuter (since these are going to be new endpoints). @turnerrainer, maybe you can clarify that this is indeed necessary.

DSL.Ruuter.private

DSL.Ruuter.public

DSL.Resql

Currently blocked because docker build for YML Ruuter fails. To reproduce:

turnerrainer commented 1 year ago

@janaliiv correct, new Ruuter services will all be based on new, YAML-based Ruuter.

turnerrainer commented 1 year ago

Ruuter build blocker can be passed by checking out to c21d3cb82943f124c777d1cef2789a0c2719db0f, we'll fix this problem later.

git checkout c21d3cb82943f124c777d1cef2789a0c2719db0f
docker-compose up -d
janaliiv commented 1 year ago

@turnerrainer, maybe you have some ideas about these problems we encountered today:

turnerrainer commented 1 year ago

This feature probably needs new config files for YML-based Ruuter (since these are going to be new endpoints). @turnerrainer, maybe you can clarify that this is indeed necessary.

I don't see a reason to provide the following endpoints:

DSL.Ruuter.private

  • cs-get-name-visibility.yml
  • cs-get-title-visibility.yml

DSL.Ruuter.public

  • get-name-visibility.yml
  • get-title-visibility.yml

DSL.Resql

  • set-name-visibility.sql
  • set-title-visibility.sql

Having them as separate endpoints seems obsolete to me as this information will be provided as part of other endpoints anyway.

turnerrainer commented 1 year ago

@turnerrainer, maybe you have some ideas about these problems we encountered today:

  • YAML Ruuter throws an error when trying to use environment variables (or any variables) in URL for outgoing requests.

As agreed upon, use hardcoded values at this point (until commiting them to our GitHub repo). This problem will be resolved soon within https://github.com/buerokratt/Ruuter/issues/99.

  • When using YAML Ruuter locally without SSL, Ruuter's security settings will not allow making a request to TIM (needed for token validation/extraction).

There's no need to use actual TIM. Create a templated Ruuter mock endpoint to provide the same JSON as TIM would in case of a successful request.

  • YAML Ruuter does not seem to handle plain string in outgoing POST request body. This is also needed for TIM.

Ruuter is not meant to make requests as plain strings but please provide additional information about where you see this as a must.

About using TIM, please see my previous comment - create and use appropriate Ruuter mock templated endpoint(s) for that.

  • No built-in validation service like in JSON Ruuter (/match_input endpoint). Couldn't find information about alternative in documentation.

You should be able to use JavaScript syntaxes within Ruuter DSLs. See https://github.com/buerokratt/Ruuter/issues/21 for more.

janaliiv commented 1 year ago

This feature probably needs new config files for YML-based Ruuter (since these are going to be new endpoints). @turnerrainer, maybe you can clarify that this is indeed necessary.

I don't see a reason to provide the following endpoints:

DSL.Ruuter.private

  • cs-get-name-visibility.yml
  • cs-get-title-visibility.yml

DSL.Ruuter.public

  • get-name-visibility.yml
  • get-title-visibility.yml

DSL.Resql

  • set-name-visibility.sql
  • set-title-visibility.sql

Having them as separate endpoints seems obsolete to me as this information will be provided as part of other endpoints anyway.

I think cs-get-name-visibility.yml and cs-get-title-visibility.yml are still needed for backoffice configuration toggles. Resql endpoints are also needed for that purpose. DSL.Ruuter.public endpoints can be skipped as we discussed last week.

janaliiv commented 1 year ago

There's no need to use actual TIM. Create a templated Ruuter mock endpoint to provide the same JSON as TIM would in case of a successful request.

@turnerrainer, I cant find an example in documentation on how to return custom JSON via Ruuter. When trying to return a javascript object using Ruuter scripting, Ruuter throws an exception.

turnerrainer commented 1 year ago

@turnerrainer, I cant find an example in documentation on how to return custom JSON via Ruuter. When trying to return a javascript object using Ruuter scripting, Ruuter throws an exception.

@janaliiv you can use call: reflect.mock for that as shown in https://github.com/buerokratt/Ruuter/blob/main/DSL/GET/steps/mock/mock-response.yml

For example, DSL

step_1:
  call: reflect.mock
  args:
    response:
      sub: ""
      firstName: "MARY ÄNN"
      idCode: "EE60001019906"
      displayName: "MARY ÄNN"
      iss: "test.buerokratt.ee"
      exp: 1670250948
      login: "EE60001019906"
      iat: 1670243748
      jti: "e14a5084-3b30-4a55-8720-c2ee22f43c2c"
      authorities: [
        "ROLE_ADMINISTRATOR"
      ]
  result: reflected_request

step_2:
  return: ${reflected_request.response.body}

would provide the following output:

{
  "response": {
    "sub": "",
    "firstName": "MARY ÄNN",
    "idCode": "EE60001019906",
    "displayName": "MARY ÄNN",
    "iss": "test.buerokratt.ee",
    "exp": 1670250948,
    "login": "EE60001019906",
    "iat": 1670243748,
    "jti": "e14a5084-3b30-4a55-8720-c2ee22f43c2c",
    "authorities": [
      "ROLE_ADMINISTRATOR"
    ]
  }
}

TIM itself would provide the following output:

{
  "sub": "",
  "firstName": "MARY ÄNN",
  "lastName": "O’CONNEŽ-ŠUSLIK TESTNUMBER",
  "idCode": "EE60001019906",
  "displayName": "MARY ÄNN",
  "iss": "test.buerokratt.ee",
  "exp": 1670250948,
  "login": "EE60001019906",
  "iat": 1670243748,
  "jti": "e14a5084-3b30-4a55-8720-c2ee22f43c2c",
  "authorities": [
    "ROLE_ADMINISTRATOR"
  ]
}

This means that the only difference is that Ruuter wraps its output into response, which is an intended and necessary result. Take this into account when creating your services.

turnerrainer commented 1 year ago

I think cs-get-name-visibility.yml and cs-get-title-visibility.yml are still needed for backoffice configuration toggles. Resql endpoints are also needed for that purpose. DSL.Ruuter.public endpoints can be skipped as we discussed last week.

Sounds good, trust you on this one.

janaliiv commented 1 year ago

Added the following Ruuter endpoints:

POST /cs-set-csa-name-visibility POST /cs-set-csa-title-visibility GET /cs-get-csa-name-visibility GET /cs-get-csa-title-visibility

Re-used set-configuration-value.sql and get-configuration.sql. Added .hbs files to convert configuration values from strings to booleans (i.e. "true" to true).

Example cURL queries (replace URL and customJwtCookie with appropriate values if needed)

curl --location --request POST 'http://localhost:9091/cs-set-csa-name-visibility' --header 'Cookie: customJwtCookie=eyJhbGciOiJSUzI1NiJ9.eyJzdWIiOiIiLCJmaXJzdE5hbWUiOiJtYXJ5IiwibGFzdE5hbWUiOiJtYXJ5IiwiaWRDb2RlIjoiRUU2MDAwMTAxOTkwNiIsImRpc3BsYXlOYW1lIjoibWFyeSIsImlzcyI6ImJ5ay5idWVyb2tyYXR0LmVlIiwiZXhwIjoxNjcwNDIzNTE2LCJsb2dpbiI6Im1hcnkiLCJpYXQiOjE2NzA0MTYzMTYsImp0aSI6IjUzZmViM2I0LWM0NTgtNGZiOC1hMmEyLTA2NzU0ZGQzYWNkZSIsImF1dGhvcml0aWVzIjpbIlJPTEVfQURNSU5JU1RSQVRPUiJdfQ.lmod2v_UmMmCxjx-Qjsv35OgwdAMWKha-cM_tejP4O5qFrMLoahUrnWxjk4rBzyI9y_4VegfPbC8nMg82QhDcGyH78N5gz2T_zatdYJZ0Ium_qD8QSyMDic2W38a0kW3Ksb4M_wiiaNSfk3vh829dvpcal0f6u2VI5Zb7iRbp3SVKkRKuhQssvQ42cyvNcw9fE8avDAwet1gBD7Ag82faHdHzWt1esDHC5aVDrUoofuAnNjACMMP3ZYLaPFOyu_hwTUHCQzDu1JUyoSzU5RmnQdFlkB0yGAyQ5dG1MYLuHDXiWlnR0cZ8huEpcIOyAPX1rAhbMQgmJQCcrgX4M8XAg' --header 'Content-Type: application/json' --data-raw '{ "isVisible": true }'

curl --location --request GET 'http://localhost:9091/cs-get-csa-name-visibility' --header 'Cookie: customJwtCookie=eyJhbGciOiJSUzI1NiJ9.eyJzdWIiOiIiLCJmaXJzdE5hbWUiOiJtYXJ5IiwibGFzdE5hbWUiOiJtYXJ5IiwiaWRDb2RlIjoiRUU2MDAwMTAxOTkwNiIsImRpc3BsYXlOYW1lIjoibWFyeSIsImlzcyI6ImJ5ay5idWVyb2tyYXR0LmVlIiwiZXhwIjoxNjcwNDIzNTE2LCJsb2dpbiI6Im1hcnkiLCJpYXQiOjE2NzA0MTYzMTYsImp0aSI6IjUzZmViM2I0LWM0NTgtNGZiOC1hMmEyLTA2NzU0ZGQzYWNkZSIsImF1dGhvcml0aWVzIjpbIlJPTEVfQURNSU5JU1RSQVRPUiJdfQ.lmod2v_UmMmCxjx-Qjsv35OgwdAMWKha-cM_tejP4O5qFrMLoahUrnWxjk4rBzyI9y_4VegfPbC8nMg82QhDcGyH78N5gz2T_zatdYJZ0Ium_qD8QSyMDic2W38a0kW3Ksb4M_wiiaNSfk3vh829dvpcal0f6u2VI5Zb7iRbp3SVKkRKuhQssvQ42cyvNcw9fE8avDAwet1gBD7Ag82faHdHzWt1esDHC5aVDrUoofuAnNjACMMP3ZYLaPFOyu_hwTUHCQzDu1JUyoSzU5RmnQdFlkB0yGAyQ5dG1MYLuHDXiWlnR0cZ8huEpcIOyAPX1rAhbMQgmJQCcrgX4M8XAg'

rasmusei commented 1 year ago

In chat widget use this figma: https://www.figma.com/file/JYnmhsKixyUSCSUXXfwTAA/B%C3%BCrokratt?type=design&node-id=3083-21452&t=khW96ClQaSGa58wG-0

janinakimtrohlev commented 1 year ago

@1AhmedYasser BUG - Right now i don't see the setting to use role or title. image There should be a setting : Under "haldus" in "vestlusrobot" tab "seaded" option "kuva nõustaja nimi" (show name) sees/väljas (on/off) is displayed Under "haldus" in "vestlusrobot" tab "seaded" option "kuva nõustaja tiitel" (show title) sees/väljas (on/off) is displayed image

1AhmedYasser commented 1 year ago

@janinakimtrohlev https://github.com/buerokratt/Buerokratt-Chatbot/issues/10#issuecomment-1711390642

This is showing on local side perfectly

(Local Environment)

local

But it looks different on dev environment, please recheck this issue on the dev environment side

(Dev Environment)

dev
janinakimtrohlev commented 1 year ago

@KlviG @varmoh In play environment I don´t see this setting.

turnerrainer commented 11 months ago

The problem is caused by a Data Mapper issue and is not related to the developments related to this issue.

Should close it as Done if the functionality is there.

@ffrose @janinakimtrohlev please re-validate.

janinakimtrohlev commented 10 months ago

Tested and done

rasmusei commented 9 months ago

@turnerrainer, @1AhmedYasser I just tested it and It's not working according to AC. I can turn on/off the functionality in the back office but it does not apply on the widget (chat window). End User won't see either title or name.

1AhmedYasser commented 9 months ago

@turnerrainer, @1AhmedYasser I just tested it and It's not working according to AC. I can turn on/off the functionality in the back office but it does not apply on the widget (chat window). End User won't see either title or name.

@rasmusei @turnerrainer according to this task thread, the only work done in it was in back-office, there is no work done on it on chat widget, i saw janaliiv added the endpoints and @Minwasko added the toggle for it on and off here https://github.com/buerokratt/Buerokratt-Chatbot/pull/202, and he mentioned the following Currently missing implementation for Chat-Widget, it needs another task on chat widget so the changes in the current back-office would be reflected in chat-widget

rasmusei commented 9 months ago

Blocked until https://github.com/buerokratt/Buerokratt-Chatbot/issues/383 is finished

janinakimtrohlev commented 8 months ago

BUG - When I put show title. Then for End User it shows as backoffice-user. But my title is "treener". So If administrator put show title, then in widget it should show CSA title.

Image

janinakimtrohlev commented 8 months ago

BUG - The same bug is when administrator choose to show name and title. Name shows normally, but title is showing wrong. Image

janinakimtrohlev commented 7 months ago

Tested and done