You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2020/04/20 23:02:41 UTC

[GitHub] [trafficcontrol] rawlinp opened a new pull request #4647: Fix /users/register DB query

rawlinp opened a new pull request #4647:
URL: https://github.com/apache/trafficcontrol/pull/4647


   ## What does this PR (Pull Request) do?
   
   The query was failing with the following:
   
       missing destination name rolename in *tc.User
   
   This indicates that the struct was missing a "db:rolename" tag for that
   field, but simply adding that tag would break the /users API due to it
   using a special struct for GetUsers.
   
   Instead, convert from sqlx.StructScan to plain sql.Scan, which avoids
   any unintended side-effects caused by changing the User struct tags.
   
   Fixes #4583.
   
   ## Which Traffic Control components are affected by this PR?
   - Traffic Ops
   
   ## What is the best way to verify this PR?
   The the TO API v1 and v2 tests, verify they still pass. Manually register a new user, verify that TO gets to at least the point where the email _would_ be sent if SMTP was enabled. If not enabled, you should see this expected error in the TO API logs instead of the DB query failure:
   
       SMTP is not enabled; mail cannot be sent
   
   That means it got past the DB error in #4583, but ideally, this would be manually verified in an environment that has SMTP enabled.
   
   ## If this is a bug fix, what versions of Traffic Control are affected?
   - master
   - 4.0.0
   
   ## The following criteria are ALL met by this PR
   
   - [x] Existing TO API tests cover most of this functionality, but they don't support email
   - [x] bugfix, no docs necessary
   - [x] This PR includes an update to CHANGELOG.md
   - [x] This PR includes any and all required license headers
   - [x] This PR ensures that database migration sequence is correct OR this PR does not include a database migration
   - [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the Apache Software Foundation's security guidelines](https://www.apache.org/security/) for details)


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficcontrol] zrhoffman removed a comment on issue #4647: Fix /users/register DB query

Posted by GitBox <gi...@apache.org>.
zrhoffman removed a comment on issue #4647:
URL: https://github.com/apache/trafficcontrol/pull/4647#issuecomment-616923014


   Unit tests pass, API tests pass, code is correctly formatted, `POST /users/register` succeeds with SMTP enabled. The response has not changed, so the docs are up-to-date.
   
   :+1: 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficcontrol] zrhoffman commented on issue #4647: Fix /users/register DB query

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on issue #4647:
URL: https://github.com/apache/trafficcontrol/pull/4647#issuecomment-616923014


   Unit tests pass, API tests pass, code is correctly formatted, `POST /users/register` succeeds with SMTP enabled. The response has not changed, so the docs are up-to-date.
   
   :+1: 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org