feat: feed submission issue creation improvement#1536
Conversation
|
We can also consider using some ts library for cleanly constructing GitHub graphql api calls - https://github.com/ts-graphql/ts-graphql. For now, this is lightweight, simple and readable enough I think? |
|
*Lighthouse ran on https://mobility-feeds-dev--pr-1536-2exn3jw4.web.app/ * (Desktop)
*Lighthouse ran on https://mobility-feeds-dev--pr-1536-2exn3jw4.web.app/feeds * (Desktop)
*Lighthouse ran on https://mobility-feeds-dev--pr-1536-2exn3jw4.web.app/feeds/gtfs/mdb-2126 * (Desktop)
*Lighthouse ran on https://mobility-feeds-dev--pr-1536-2exn3jw4.web.app/feeds/gtfs_rt/mdb-2585 * (Desktop)
*Lighthouse ran on https://mobility-feeds-dev--pr-1536-2exn3jw4.web.app/gbfs/gbfs-flamingo_porirua * (Desktop)
|
|
Preview Firebase Hosting URL: https://mobility-feeds-dev--pr-1536-2exn3jw4.web.app |
|
Got the scopes updated with the help of @fredericsimard 🙌 It is adding issues to the Feed Submissions Backlog Project as intended and now ready for review. |
| const projectId = "PVT_kwDOAnHxDs4Ayxl6"; | ||
| const statusFieldId = "PVTSSF_lADOAnHxDs4Ayxl6zgorIUI"; | ||
| const backlogOptionId = "8e14ac56"; |
There was a problem hiding this comment.
These hardcoded variables should be hidden as ENV variables ex
const sheetId = process.env.FEED_SUBMIT_GOOGLE_SHEET_ID;
There was a problem hiding this comment.
I agree, we could also use 1Password to store them, there's an SDK for Javascript.
| /** | ||
| * Parses the provided URL to check if it is a valid ZIP file URL | ||
| * @param {string | undefined | null } url The direct download URL | ||
| * @return {boolean} Whether the URL is a valid ZIP file URL | ||
| */ | ||
| function isValidZipUrl(url: string | undefined | null): boolean { | ||
| if (!url) return false; | ||
| try { | ||
| const parsed = new URL(url); | ||
| return parsed.pathname.toLowerCase().endsWith(".zip"); | ||
| } catch { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Checks if URL points to a valid ZIP file by making HEAD request | ||
| * @param {string | undefined | null } url The download URL | ||
| * @return {boolean} Whether the URL downloads a valid ZIP file | ||
| */ | ||
| async function isValidZipDownload( | ||
| url: string | undefined | null | ||
| ): Promise<boolean> { | ||
| try { | ||
| if (!url) return false; | ||
| const response = await axios.head(url, {maxRedirects: 2}); | ||
| const contentType = response.headers["content-type"]; | ||
| const contentDisposition = response.headers["content-disposition"]; | ||
|
|
||
| if (contentType && contentType.includes("zip")) return true; | ||
| if (contentDisposition && contentDisposition.includes("zip")) return true; | ||
| return false; | ||
| } catch { | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
As this file is getting large, bring these functions out into a helper file ex functions/packages/feed-form/src/impl/utils.ts and writing unit tests for them would be good
| * @param {string} statusFieldId The ID of the Status field | ||
| * @param {string} statusOptionId The ID of the status option | ||
| */ | ||
| async function addIssueToProjectV2( |
There was a problem hiding this comment.
same idea with this function, bring it outside
| labels.push("auth required"); | ||
| } | ||
|
|
||
| if (!isValidZipUrl(formData.feedLink)) { |
There was a problem hiding this comment.
For GTFS feeds, the expectation is that the resource's content is a ZIP file. However, many URLs don't have a zip suffix. I would remove this condition in favor of data_type == GTFS and not authentication required (since the key will not be available when we check the resource).
4 participants
Summary:
Extend on #1477 PR to:
Expected behavior:
New behaviour:
Upon submission of a new feed, the direct download URL should be validated to check that it produces a zip file. If the URL is invalid then we should see the "invalid" tag added to the issue. The issue should also be visible in the "Backlog" column of the Feed Submissions Backlog Project. Previously it did not belong to any project but now it should be added upon issue creation.
Note: The personal access token used for creating GitHub issues does not appear to have the required scopes/permissions. When I tested using the graphql api just using curl, it states:
Before reviewing and merging the PR, I need to clarify I'm using the right token/if there's a different token I should use. Most likely, we need to add the appropriate scopes to the same token so that the graphql api calls will work! cc. @davidgamez and @fredericsimard .
Testing tips:
It can be tested by filling out the feed submission form on the website and verifying that the issue created is showing up in the Feed Submissions Backlog Project. It can be further tested by providing some URL that doesn't produce a valid zip file, which should result in the issue having the 'invalid' label.
Please make sure these boxes are checked before submitting your pull request - thanks!
./scripts/api-tests.shto make sure you didn't break anything