Police-Data-Accessibility-Project / data-sources-app

An API and UI for using and maintaining the Data Sources database
MIT License
2 stars 5 forks source link

v2: requests support for back end #256

Open josh-chamberlain opened 4 months ago

josh-chamberlain commented 4 months ago

Context

part of #254 part of #32

Requirements

mbodeantor commented 4 months ago

@josh-chamberlain Do we want a separate Github project for requests for to funnel them into the current data sources app one?

josh-chamberlain commented 4 months ago

@mbodeantor definitely a separate project, I made one here and main comment adjusted

maxachis commented 4 months ago

@josh-chamberlain Navigating Airtable is still a challenge for me -- could you point me to the relevant Airtable for requests?

maxachis commented 4 months ago

@josh-chamberlain For the data_requests table in PostgreSQL, I'd like to make the following modifications in addition to developing the endpoints:

  1. Change status_last_changed to date_last_changed, for clarity
  2. Add triggers to automatically update the date_created and date_last_changed columns when the relevant actions occur.
  3. Replace request_status's text type with an enum to enforce the permitted statuses (Intake, Active, Complete, Request withdrawn, Waiting for scraper, Archived)
  4. Have all newly created rows have request_status be "Intake" by default
mbodeantor commented 4 months ago

Here is the Airtable Requests table: https://airtable.com/app473MWXVJVaD7Es/tblV2ftkAwaNJfDTJ/viwtwWW7t6Rwaev8X?blocks=hide

mbodeantor commented 4 months ago

Part of the reasoning behind replacing Airtable is to not have to make changes in multiple places (Airtable itself, the script mirroring Airtable to Postgres, Postgres itself). Not sure it makes sense to start make these changes before we move off of Airtable for tracking requests.

josh-chamberlain commented 4 months ago

@maxachis @mbodeantor yep, this issue = replacing Airtable for requests.

  1. Change status_last_changed to date_last_changed, for clarity
    • status_last_changed only updates when the status property changes, does that make the name make more sense?
  2. Add triggers to automatically update the date_created and date_last_changed columns when the relevant actions occur.
    • yep, Airtable handles this automatically but we'd need to
  3. Replace request_status's text type with an enum to enforce the permitted statuses (Intake, Active, Complete, Request withdrawn, Waiting for scraper, Archived)
    • makes sense
  4. Have all newly created rows have request_status be "Intake" by default
    • makes sense
mbodeantor commented 4 months ago

@josh-chamberlain @maxachis okay just wanted to be clear that these changes are happening to a new requests table and not the current one

josh-chamberlain commented 4 months ago

@mbodeantor whatever's best from your perspective, I could see an argument either way. Your call

maxachis commented 4 months ago
  • status_last_changed only updates when the status property changes, does that make the name make more sense?

@josh-chamberlain In which case, I'd like to rename to date_status_last_changed just to make it extra super-duper clear 🙃. It's a minor thing, but I figure if it threw me off it'd throw others off as well. I can add a trigger for that as well.

maxachis commented 3 months ago

@josh-chamberlain Do we want to enforce the form of contact information? Looking at the current submitter_contact_info column, a number of the submissions given are of limited use (some just give names or contextless phone numbers, one even simply says "Mail". Maybe break it down into name, email, and phone number, but also allow them to select an anonymous option?

maxachis commented 3 months ago

Current proposed design

josh-chamberlain commented 3 months ago

@maxachis

maxachis commented 3 months ago

@josh-chamberlain Acknowledged! Design updated, and PR for table creation has been created.

maxachis commented 3 months ago
  • allow people to click "interested" on a request and associate it with their profile

In addition to the above table design, this would likely require an additional linking table to express this relationship. Probably called something like user_interested_data_request

maxachis commented 3 months ago

@josh-chamberlain Here is the current structure of the post request, which also details my expectations for what the front end will provide:

    @api_required
    @handle_exceptions
    def post(self) -> tuple[dict, int]:
        """
        Handles the creation of a new data request.
        Expects 'submission_notes' and 'submitter_contact_info' in the request body.
        """
        request_info = RequestInfo(
            submission_notes=request.json["submission_notes"],
            submitter_contact_info=request.json["submitter_contact_info"],
            submitter_user_id=request.json["submitter_user_id"],
            agency_described_submitted=request.json["agency_described_submitted"],
            record_type=str_to_enum(RecordType, request.json["record_type"])

        )
        record_id = self.data_manager.create_request(
            request_info
        )
        return {"message": "Created successfully", "id": record_id}, 201

My questions:

  1. Should I expect this to be the relevant information provided by the frontend?
  2. The post request currently expects all data requests to be submitted. Are there times where we'd want to reject a request, such as if an identical request already exists?
maxachis commented 3 months ago

@josh-chamberlain Additionally, what data should be returned on a GET/Read request for a single record? For clarity, here is the current design of the table:

CREATE TABLE requests_v2 (
    id BIGSERIAL PRIMARY KEY,
    submission_notes TEXT NOT NULL,
    request_status request_status NOT NULL DEFAULT 'Intake',
    submitter_contact_info TEXT,
    submitter_user_id BIGINT REFERENCES users(id),
    agency_or_area_described NOT NULL TEXT,
    record_type record_type,
    archive_reason TEXT,
    date_created TIMESTAMP NOT NULL DEFAULT NOW(),
    date_status_last_changed TIMESTAMP NOT NULL DEFAULT NOW(),
    github_issue_url TEXT CHECK (github_issue_url IS NULL OR github_issue_url ~* '^https?://[^\s/$.?#].[^\s]*$')
);
josh-chamberlain commented 3 months ago

@maxachis

  1. small note: we should probably require "agency or area described"
  2. yes, I think so—attached is a rough screenshot. "project description" is probably just "submission notes".
  3. let's not worry about rejecting them unless they're missing required info; at this point they're all going to be human-reviewed anyway.
Screen Shot 2024-05-17 at 1 12 13 PM
maxachis commented 3 months ago

@josh-chamberlain

Read/Get

To make sure I'm understanding properly, will each get request return:

  1. submission_notes
  2. agency_described_submitted
  3. record_type
  4. Not Github Issue URL?

Are we additionally returning other information (i.e., the Tasks and Relevant Agencies, if any?). I'm not sure if we have the table set up for that yet.

Update/Put

Will put be used by the user, or by us? In either case, what attributes do we want to be able to modify?

josh-chamberlain commented 3 months ago

@maxachis

  1. yes, we'll use github issue URL where #NNN is in the screenshot.

Don't worry about tasks and relevant agencies for now—we can iterate from a basic endpoint, which will get us very deep into prototyping.

I peeled off the github integration as a separate issue, plus there's auth. I wonder if we should be scoping those out + progressing there before making it too far down this path anyway?

josh-chamberlain commented 2 months ago

@maxachis this one is still not at the top of the list or anything, but for posterity: I made many updates to the parent issue reflecting much of what we have discussed.

maxachis commented 1 week ago

@josh-chamberlain When users specify the location for a request, I'm wondering if it would be useful to have them select from locations in a manner similar (or identical to) our /typeahead_suggestions endpoint. i.e., they type in the location, and we generate location suggestions for them. Just a thought.

josh-chamberlain commented 1 week ago

I left a comment on the wireframe to that effect, but I realize I failed to share the theming wireframe (most up to date)

Screen Shot 2024-08-21 at 2 52 56 PM