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

api: expose journalist first and last name if set #4459

Merged
merged 1 commit into from
Jun 4, 2019

Conversation

redshiftzero
Copy link
Contributor

@redshiftzero redshiftzero commented May 23, 2019

Status

Ready for review

Description of Changes

This is a followup from #4425. In this PR we expose the journalist first name and last name added in #4425 on:

  • the user endpoint
  • the reply endpoints (since this is the place where we want to use them in the client, for each conversation)

Testing

Reading the diff and tests should be sufficient, but here are steps to test an endpoint that has the first name / last name added with curl for convenience:

  1. Get a token curl -X POST -H "Content-Type: application/json" --data '{"username":"journalist","passphrase":"correct horse battery staple profanity oil chewy","one_time_code":"ADDYOURSHERE"}' 127.0.0.1:8081/api/v1/token

  2. Get curl -X GET -H "Content-Type: application/json" -H "Authorization: Token ADDYOURSHERE" 127.0.0.1:8081/api/v1/user

{
  "first_name": "",
  "is_admin": true,
  "last_login": "2019-05-23T19:07:13.967833Z",
  "last_name": "",
  "username": "journalist",
  "uuid": "e1f5636c-d141-4cac-bc6d-7ed1e6d06dae"
}

Deployment

No special considerations

Checklist

If you made changes to the server application code:

  • Linting (make lint) and tests (make -C securedrop test) pass in the development container (I'm relying on CI, only ran the API tests)

If you made non-trivial code changes:

  • I have written a test plan and validated it for this PR

If you made changes to documentation:

  • Doc linting (make docs-lint) passed locally

@codecov-io
Copy link

Codecov Report

Merging #4459 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #4459   +/-   ##
========================================
  Coverage    83.21%   83.21%           
========================================
  Files           45       45           
  Lines         3069     3069           
  Branches       332      332           
========================================
  Hits          2554     2554           
  Misses         430      430           
  Partials        85       85
Impacted Files Coverage Δ
securedrop/securedrop/models.py 89.22% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 537dd23...cb8b644. Read the comment docs.

@rmol
Copy link
Contributor

rmol commented May 24, 2019

👍 Tested these endpoints as well as the reply endpoints, to check the doc changes. All looks good.

securedrop/models.py Outdated Show resolved Hide resolved
securedrop/models.py Outdated Show resolved Hide resolved
kushaldas
kushaldas previously approved these changes May 28, 2019
Copy link
Contributor

@kushaldas kushaldas left a comment

Choose a reason for hiding this comment

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

This is good, thank you 🌈 🌈 🌈

we expose them on:
 - the user endpoint
 - the reply JSON objects (since this is the place where we want
 to use them in the client)
@redshiftzero
Copy link
Contributor Author

hey @kushaldas this needs a restamp (I did make a change above so your review was dismissed)

Copy link
Contributor

@kushaldas kushaldas left a comment

Choose a reason for hiding this comment

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

Approved.

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.

5 participants