Skip to content

Commit

Permalink
Rest api design (#601)
Browse files Browse the repository at this point in the history
* Bump zod from 3.22.2 to 3.22.3 in /client

Bumps [zod](https://github.com/colinhacks/zod) from 3.22.2 to 3.22.3.
- [Release notes](https://github.com/colinhacks/zod/releases)
- [Changelog](https://github.com/colinhacks/zod/blob/master/CHANGELOG.md)
- [Commits](colinhacks/zod@v3.22.2...v3.22.3)

---
updated-dependencies:
- dependency-name: zod
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>

* remove unused manifest viewset endpoints (PUT, PATCH, DELETE, POST)

our logic for how we interact with the manifest not really appropriate for a DRF model ViewSet. so we had a bunch of unused endpoints that was autoamtically created by DRF when we really just needed to RetreiveAPIView

* prefix trak URLs (manifest, waste codes, etc.) with rcra to better establish the resource hierarchy in the RESTful api design

we want to establish that these are RCRA related resources that are not central to the haztrak application itself or another environmental act like the clean water act (CWA) which may have their own 'manifest' or definition of an 'EPA site'

* reconfigure URL to point to new trak urls that a re prefixed with 'rcra/'

* convert user/* endpoints related to the user's RCRA profile to rcra/profile* endpoints

* update test suite to use new rcra/* based URLs

* change rcra-site to handler and prefix with rcra/*

* add schema url prefix with optional 'rcra/' regex group

* remove redundant handler function for retrieving a site's manifest tracking numbers

* convert Handler search to class based function, add OpenAPI schema auto generated docs

* update our schema documentation for various views

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
  • Loading branch information
dpgraham4401 and dependabot[bot] authored Oct 10, 2023
1 parent 138af2f commit e41fe1e
Show file tree
Hide file tree
Showing 27 changed files with 168 additions and 237 deletions.
8 changes: 4 additions & 4 deletions client/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
"react-router-dom": "^6.16.0",
"react-select": "^5.7.7",
"sass": "^1.68.0",
"zod": "^3.22.2"
"zod": "^3.22.3"
},
"devDependencies": {
"@testing-library/jest-dom": "^6.1.3",
Expand Down
2 changes: 1 addition & 1 deletion client/src/components/Manifest/ManifestForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ export function ManifestForm({
const onSubmit: SubmitHandler<Manifest> = (data: Manifest) => {
console.log('Manifest Submitted', data);
htApi
.post<TaskStatus>('/rcra/manifest/create', data)
.post<TaskStatus>('/rcra/manifest', data)
.then((response) => {
return response;
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export function QuickerSignForm({ mtn, mtnHandler, handleClose, siteType }: Quic
};
}
htApi
.post('/manifest/sign', signature)
.post('rcra/manifest/sign', signature)
.then((response: AxiosResponse) => {
dispatch(
addNotification({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const testTaskID = 'testTaskId';

const server = setupServer(
...[
rest.post(`${API_BASE_URL}/manifest/sync`, (req, res, ctx) => {
rest.post(`${API_BASE_URL}rcra/manifest/sync`, (req, res, ctx) => {
// Mock Sync Site Manifests response
return res(
ctx.status(200),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { faSync } from '@fortawesome/free-solid-svg-icons';
import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
import { RcraApiUserBtn } from 'components/buttons';
import React, { useState } from 'react';
import { htApi } from 'services';
import { RcraApiUserBtn } from 'components/buttons';
import { addNotification, useAppDispatch } from 'store';

interface SyncManifestProps {
Expand All @@ -18,8 +18,7 @@ interface SyncManifestProps {
export function SyncManifestBtn({ siteId, disabled }: SyncManifestProps) {
const [syncingMtn, setSyncingMtn] = useState(false);
const dispatch = useAppDispatch();
let active: boolean = siteId ? true : false;
active = !disabled;
const active = !disabled;

return (
<RcraApiUserBtn
Expand All @@ -29,7 +28,7 @@ export function SyncManifestBtn({ siteId, disabled }: SyncManifestProps) {
onClick={() => {
setSyncingMtn(!syncingMtn);
htApi
.post('/manifest/sync', { siteId: `${siteId}` })
.post('rcra/manifest/sync', { siteId: `${siteId}` })
.then((response) => {
dispatch(
addNotification({
Expand Down
4 changes: 2 additions & 2 deletions client/src/features/home/Home.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
import '@testing-library/jest-dom';
import { Home } from './Home';
import { rest } from 'msw';
import { setupServer } from 'msw/node';
import React from 'react';
import { cleanup, renderWithProviders, screen } from 'test-utils';
import { API_BASE_URL, handlers } from 'test-utils/mock/handlers';
import { afterAll, afterEach, beforeAll, describe, expect, test, vi } from 'vitest';
import { Home } from './Home';

const USERNAME = 'testuser1';

const myAPIHandlers = [
rest.get(`${API_BASE_URL}/api/user/${USERNAME}/rcra/profile`, (req, res, ctx) => {
rest.get(`${API_BASE_URL}/api/rcra/profile/${USERNAME}`, (req, res, ctx) => {
return res(
// Respond with a 200 status code
ctx.status(200),
Expand Down
2 changes: 1 addition & 1 deletion client/src/features/manifestDetails/ManifestDetails.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { useParams } from 'react-router-dom';
export function ManifestDetails() {
const { mtn, action, siteId } = useParams();
useTitle(`${mtn}`);
const [manifestData, loading, error] = useHtApi<Manifest>(`manifest/${mtn}`);
const [manifestData, loading, error] = useHtApi<Manifest>(`rcra/manifest/${mtn}`);

let readOnly = true;
if (action === 'edit') {
Expand Down
4 changes: 2 additions & 2 deletions client/src/features/manifestList/ManifestList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ export function ManifestList(): ReactElement {
useTitle(`${siteId || ''} Manifest`);
const [pageSize, setPageSize] = useState(10);

let getUrl = 'mtn';
let getUrl = 'rcra/mtn';
if (siteId) {
getUrl = `mtn/${siteId}`;
getUrl = `rcra/mtn/${siteId}`;
}

const [mtnList, loading, error] = useHtApi<Array<MtnDetails>>(getUrl);
Expand Down
7 changes: 3 additions & 4 deletions client/src/features/notifications/Notifications.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { faFaceSmile } from '@fortawesome/free-solid-svg-icons';
import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
import winkingRobot from '/static/robot-wink.jpg';
import { HtCard } from 'components/Ht';
import { NotificationRow } from 'components/Notification';
import { useTitle } from 'hooks';
Expand All @@ -25,8 +24,8 @@ export function Notifications() {
<HtCard.Body>
{notifications.length === 0 ? (
<div className="text-center">
<h3>Nothing to see here, you're all caught up!</h3>
<FontAwesomeIcon className="text-info" icon={faFaceSmile} size={'6x'} />
<h3>You're all caught up!</h3>
<img src={winkingRobot} alt="happy robot" width={200} height={'auto'} />
</div>
) : (
<Table hover>
Expand Down
6 changes: 3 additions & 3 deletions client/src/features/profile/RcraProfile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ import { HtForm } from 'components/Ht';
import React, { useState } from 'react';
import { Button, Col, Container, Form, Row, Table } from 'react-bootstrap';
import { useForm } from 'react-hook-form';
import { Link } from 'react-router-dom';
import { htApi } from 'services';
import { addNotification, useAppDispatch } from 'store';
import { getProfile, updateProfile } from 'store/rcraProfileSlice';
import { RcraProfileState } from 'store/rcraProfileSlice/rcraProfile.slice';
import { htApi } from 'services';
import { Link } from 'react-router-dom';
import { z } from 'zod';

interface ProfileViewProps {
Expand Down Expand Up @@ -48,7 +48,7 @@ export function RcraProfile({ profile }: ProfileViewProps) {
setProfileLoading(!profileLoading);
setEditable(!editable);
htApi
.put(`/user/${profile.user}/rcra/profile`, data)
.put(`/rcra/profile/${profile.user}`, data)
.then((r) => {
dispatch(updateProfile(r.data));
})
Expand Down
2 changes: 1 addition & 1 deletion client/src/store/rcraProfileSlice/rcraProfile.slice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ export const getProfile = createAsyncThunk<RcraProfileState>(
async (arg, thunkAPI) => {
const state = thunkAPI.getState() as RootState;
const username = state.user.user?.username;
const response = await htApi.get(`/user/${username}/rcra/profile`);
const response = await htApi.get(`/rcra/profile/${username}`);
const { rcraSites, ...rest } = response.data as RcraProfileResponse;
// Convert the array of RcraSite permissions we get from our backend
// to an object which each key corresponding to the RcraSite's ID number
Expand Down
8 changes: 3 additions & 5 deletions client/src/store/site.slice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,24 +14,22 @@ interface RcrainfoSiteSearch {
export const siteApi = createApi({
reducerPath: 'siteApi',
baseQuery: htApiBaseQuery({
baseUrl: `${import.meta.env.VITE_HT_API_URL}/api/site`,
baseUrl: `${import.meta.env.VITE_HT_API_URL}/api/`,
}),
endpoints: (build) => ({
searchRcrainfoSites: build.query<Array<RcraSite>, RcrainfoSiteSearch>({
query: (data: RcrainfoSiteSearch) => ({
url: '/rcrainfo/search',
url: 'rcra/handler/search',
method: 'post',
data: data,
}),
}),
searchRcraSites: build.query<Array<RcraSite>, RcrainfoSiteSearch>({
query: (data: RcrainfoSiteSearch) => ({
url: '/search',
url: 'site/search',
method: 'get',
params: { epaId: data.siteId, siteType: data.siteType },
}),
}),
}),
});

export const { useSearchRcraSitesQuery, useSearchRcrainfoSitesQuery } = siteApi;
2 changes: 1 addition & 1 deletion client/src/store/wasteCode.slice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { htApiBaseQuery } from 'store/baseQuery';
export const wasteCodeApi = createApi({
reducerPath: 'wasteCodeApi',
baseQuery: htApiBaseQuery({
baseUrl: `${import.meta.env.VITE_HT_API_URL}/api/code/`,
baseUrl: `${import.meta.env.VITE_HT_API_URL}/api/rcra/code/`,
}),
endpoints: (build) => ({
getFedWasteCodes: build.query<Array<Code>, void>({
Expand Down
6 changes: 3 additions & 3 deletions client/src/test-utils/mock/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export const handlers = [
/**
* User RcraProfile data
*/
rest.get(`${API_BASE_URL}/api/user/${mockUsername}/rcra/profile`, (req, res, ctx) => {
rest.get(`${API_BASE_URL}/api/rcra/profile/${mockUsername}`, (req, res, ctx) => {
return res(
// Respond with a 200 status code
ctx.status(200),
Expand Down Expand Up @@ -57,13 +57,13 @@ export const handlers = [
/**
* mock Manifest
*/
rest.get(`${API_BASE_URL}/api/manifest/${mockMTN}`, (req, res, ctx) => {
rest.get(`${API_BASE_URL}/api/rcra/manifest/${mockMTN}`, (req, res, ctx) => {
return res(ctx.status(200), ctx.json(createMockManifest()));
}),
/**
* list of manifests ('My Manifests' feature and a site's manifests)
*/
rest.get(`${API_BASE_URL}/api/mtn*`, (req, res, ctx) => {
rest.get(`${API_BASE_URL}/api/rcra/mtn*`, (req, res, ctx) => {
const mockManifestArray = [
createMockManifest(),
createMockManifest({ manifestTrackingNumber: '987654321ELC', status: 'Pending' }),
Expand Down
15 changes: 11 additions & 4 deletions server/apps/core/urls.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from django.urls import path
from django.urls import include, path

from .views import (
from .views import ( # type: ignore
HaztrakUserView,
LaunchExampleTaskView,
Login,
Expand All @@ -11,8 +11,15 @@

urlpatterns = [
# Rcra Profile
path("user/<str:username>/rcra/profile/sync", RcraProfileSyncView.as_view()),
path("user/<str:username>/rcra/profile", RcraProfileView.as_view()),
path(
"rcra/",
include(
[
path("profile/<str:username>/sync", RcraProfileSyncView.as_view()),
path("profile/<str:username>", RcraProfileView.as_view()),
]
),
),
path("user", HaztrakUserView.as_view()),
path("user/login", Login.as_view()),
path("task/example", LaunchExampleTaskView.as_view()),
Expand Down
9 changes: 4 additions & 5 deletions server/apps/sites/tests/test_epa_profile_views.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,16 @@
import pytest
from rest_framework import status
from rest_framework.response import Response
from rest_framework.test import APIRequestFactory, force_authenticate

from apps.core.views import RcraProfileView
from apps.core.views import RcraProfileView # type: ignore


class TestRcraProfileView:
"""
Tests the for the endpoints related to the user's RcraProfile
"""

URL = "/api/user"
URL = "/api/"
id_field = "rcraAPIID"
key_field = "rcraAPIKey"
username_field = "rcraUsername"
Expand All @@ -28,7 +27,7 @@ def user_and_client(self, rcra_profile_factory, user_factory, api_client_factory
def rcra_profile_request(self, user_and_client):
factory = APIRequestFactory()
request = factory.put(
f"{self.URL}/{self.user.username}/rcra/profile",
f"{self.URL}rcra/profile{self.user.username}",
{
self.id_field: self.new_api_id,
self.username_field: self.new_username,
Expand All @@ -43,7 +42,7 @@ def test_returns_a_user_profile(self, user_and_client, rcra_profile_factory):
# Arrange
rcra_profile_factory(user=self.user)
# Act
response: Response = self.client.get(f"{self.URL}/{self.user.username}/rcra/profile")
response = self.client.get(f"{self.URL}rcra/profile/{self.user.username}")
# Assert
assert response.headers["Content-Type"] == "application/json"
assert response.status_code == status.HTTP_200_OK
Expand Down
6 changes: 3 additions & 3 deletions server/apps/sites/tests/test_epa_site_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,16 @@
from rest_framework.response import Response
from rest_framework.test import APIClient, APIRequestFactory, force_authenticate

from apps.sites.models import RcraSiteType
from apps.sites.views import SiteSearchView
from apps.sites.models import RcraSiteType # type: ignore
from apps.sites.views import SiteSearchView # type: ignore


class TestEpaSiteView:
"""
Tests the for the endpoints related to the handlers
"""

URL = "/api/site/rcra-site"
URL = "/api/rcra/handler"

@pytest.fixture
def client(self, rcra_site_factory, api_client_factory):
Expand Down
34 changes: 4 additions & 30 deletions server/apps/sites/tests/test_site_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,11 @@
import pytest
from django.contrib.auth.models import User
from rest_framework import status
from rest_framework.response import Response
from rest_framework.test import APIClient, APIRequestFactory, force_authenticate

from apps.core.models import RcraProfile
from apps.sites.models import RcraSite, RcraSitePermission, Site
from apps.sites.views import SiteDetailView, SiteMtnListView
from apps.core.models import RcraProfile # type: ignore
from apps.sites.models import RcraSite, RcraSitePermission, Site # type: ignore
from apps.sites.views import SiteDetailView # type: ignore


class TestSiteListView:
Expand Down Expand Up @@ -96,7 +95,7 @@ def create_site_and_related(

return create_site_and_related

def test_returns_site_by_id(self, user_factory, local_site_factory):
def test_returns_site_by_id(self, user_factory, local_site_factory) -> None:
# Arrange
user = user_factory(username="username1")
site = local_site_factory(user=user)
Expand Down Expand Up @@ -132,28 +131,3 @@ def test_returns_formatted_http_response(self, user_factory, local_site_factory,
# Assert
assert response.headers["Content-Type"] == "application/json"
assert response.status_code == status.HTTP_200_OK


class TestSiteManifest:
"""
Tests for the endpoint to retrieve a Site's manifests
"""

# ToDo fix these false positive tests

url = "/api/site"

@pytest.fixture(autouse=True)
def _user(self, user_factory):
self.user = user_factory()

@pytest.fixture(autouse=True)
def _generator(self, rcra_site_factory):
self.generator = rcra_site_factory()

def test_returns_200(self):
factory = APIRequestFactory()
request = factory.get(f"{self.url}/{self.generator.epa_id}/manifest")
force_authenticate(request, self.user)
response: Response = SiteMtnListView.as_view()(request, self.generator.epa_id)
print(response.data)
Loading

0 comments on commit e41fe1e

Please sign in to comment.