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

feat(api): add external API endpoint for creating urls #2069

Merged
merged 8 commits into from
Nov 15, 2022

Conversation

halfwhole
Copy link
Collaborator

@halfwhole halfwhole commented Nov 3, 2022

Problem

We want to add an external API endpoint for creating URLs (POST /api/v1/urls)

See Notion page on external API

Solution

Features:

  • Created a new ApiV1Controller in src/server/modules/api/external-v1, with the createUrl method
  • Request fields to create URL: shortUrl (optional), longUrl
  • Response fields for created URL (as defined by UrlV1DTO type): shortUrl, longUrl, state, clicks, createdAt, updatedAt - note that certain fields e.g. tags, description have been restricted
    • To map from the original DTO StorableUrl to UrlV1DTO, I wrote a private function sanitizeUrlForApi in the controller that picks out only the fields that we want to return - is this okay, or is there a better method? Will be expecting this function to be reused in the future for other parts of the API controller
    • Decided against creating a new mapper, because mappers seem specific to transforming from persistent -> DTO
    • EDIT: we'll create a new mapper UrlV1Mapper to map from StorableUrl to UrlV1DTO, which should do for now
  • UrlManagementService and UrlRepository have been changed to additionally take in a source field, to distinguish between console vs. API created links
  • Added a fallback 404 for all non-matching /api/v1/<...> routes

Improvements:

  • Made a number of small edits to the error messages, this may clutter the files changed (sorry!)
  • The backend currently returns the error message Short link "<shortUrl>" is used. Click here to find out more when the short URL is already taken. But it seems weird to return "Click here to find out more" from the backend - the external API definitely shouldn't have that, so I've removed it from the external API but preserved it in the internal API. In the future, we should shift the "Click here" part to the frontend

Future possible improvements:

  • Change internal API endpoint from /api/user/url to /api/user/urls, consistent with the external API /api/v1/urls
  • Restrict user from submitting userId in request body when creating URL through external API

Tests

  • Unit tests for API controller - these borrow heavily from the unit tests for the original User controller

Should write integration tests too, when we have them

Deploy notes

New environment variables:

  • (Optional) API_LINK_RANDOM_STR_LENGTH: length of randomly generated shortUrl for API created links, defaults to 8

@halfwhole halfwhole marked this pull request as ready for review November 7, 2022 06:27
@halfwhole halfwhole requested a review from thanhdatle November 7, 2022 06:27
src/server/modules/api/external-v1/ApiController.ts Outdated Show resolved Hide resolved
src/server/modules/api/external-v1/index.ts Outdated Show resolved Hide resolved
src/server/modules/user/UserController.ts Outdated Show resolved Hide resolved
@halfwhole halfwhole requested a review from thanhdatle November 9, 2022 06:41
@gitguardian

This comment was marked as resolved.

@halfwhole
Copy link
Collaborator Author

halfwhole commented Nov 10, 2022

^ ignore the above gitguardian warning, it's from the previously-defined API_KEY_SALT in docker-compose.yml which was not even touched by this PR

Realised that shortUrl should be optional instead of required for API-created URLs, so just added it in a5815c6. The randomly generated shortUrl has a length according to the optional env variable API_LINK_RANDOM_STR_LENGTH, defaults to 8 (similar to bulk created links)

@halfwhole halfwhole merged commit b30b5fe into develop Nov 15, 2022
@halfwhole halfwhole deleted the feat/api/create-url branch November 15, 2022 03:13
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.

2 participants