Skip to content

Conversation

@m0nggh
Copy link
Contributor

@m0nggh m0nggh commented Nov 19, 2024

Details

  • Add warning when deleting connections
  • Remove duplicate text in dropdowns e.g. custom api
  • Catch postman attachment size exceeded error

kevinkim-ogp and others added 5 commits November 11, 2024 13:07
## Problem
Duplicate text shown in dropdown for Custom API Method.



Closes
https://linear.app/ogp/issue/PLU-360/remove-duplicate-text-in-method-for-custom-api

## Solution
Set showOptionValue to false to hide the value the dropdown options.

## Before & After Screenshots

**BEFORE**:
<img width="853" alt="Screenshot 2024-11-08 at 9 54 57 AM"
src="https://github.com/user-attachments/assets/1868ffdb-56ba-4921-b62c-abb1b304aedc">

**AFTER**:
<img width="853" alt="Screenshot 2024-11-08 at 11 30 52 AM"
src="https://github.com/user-attachments/assets/6121c038-36b3-4af0-a3e5-65ccf973a868">


## Tests
- [x] Able to execute new api calls based on selection
- [x] Able to update method for existing custom api
## Problem
Dropdown options showing unnecessary values.

Closes [PLU-362:
remove-duplicate-text-in-dropdowns](https://linear.app/ogp/issue/PLU-362/remove-duplicate-text-in-dropdowns)

## Solution
Set showOptionValue to false to hide duplicate text in dropdown

## Before & After Screenshots

**BEFORE**:
- Delay
<img width="852" alt="Screenshot 2024-11-14 at 9 51 42 AM"
src="https://github.com/user-attachments/assets/93fa8f63-e6fc-4140-bec4-5fe33f73715f">

- Scheduler
<img width="858" alt="Screenshot 2024-11-14 at 9 52 45 AM"
src="https://github.com/user-attachments/assets/0955e01a-ec4d-4009-9efe-f499041dfb73">
<img width="856" alt="Screenshot 2024-11-14 at 9 53 15 AM"
src="https://github.com/user-attachments/assets/154b833c-0bd9-42e2-ae82-912415746a57">
<img width="851" alt="Screenshot 2024-11-14 at 9 54 26 AM"
src="https://github.com/user-attachments/assets/4d09d46c-83b9-4c4a-b7b0-f2ed76dcf5ed">


- Slack
<img width="856" alt="Screenshot 2024-11-14 at 9 50 42 AM"
src="https://github.com/user-attachments/assets/125a819a-3d8b-47c2-b8c1-1220b49fbdfd">
<img width="853" alt="Screenshot 2024-11-14 at 9 50 55 AM"
src="https://github.com/user-attachments/assets/5b64fe29-82ab-452f-b935-ccc264349a53">


**AFTER**:
- Delay
<img width="852" alt="Screenshot 2024-11-14 at 9 51 56 AM"
src="https://github.com/user-attachments/assets/5fa9cc62-ab02-4c78-a77d-2a002f126a12">

- Scheduler
<img width="855" alt="Screenshot 2024-11-14 at 9 52 57 AM"
src="https://github.com/user-attachments/assets/14b02873-e97f-4614-87c2-76a0ebf783de">
<img width="855" alt="Screenshot 2024-11-14 at 9 53 29 AM"
src="https://github.com/user-attachments/assets/93813d20-e9af-454c-976b-c12494100b2e">
<img width="855" alt="Screenshot 2024-11-14 at 9 54 53 AM"
src="https://github.com/user-attachments/assets/3c06e1a0-e461-409b-9b4c-537f3d1a9267">

- Slack
<img width="857" alt="Screenshot 2024-11-14 at 9 49 43 AM"
src="https://github.com/user-attachments/assets/a33c2480-506d-4eff-865f-2f1d57a25c27">
<img width="856" alt="Screenshot 2024-11-14 at 9 49 53 AM"
src="https://github.com/user-attachments/assets/d14eab37-d7bc-4dbd-8775-694ac55bc197">

## Tests
- [x] Action values saved correctly
- [x] Existing steps show the correct options
## Problem

Currently, we don't check for attachment exceeded error for postman
step.
Postman
[guide](https://postman-v1.guides.gov.sg/email-api-guide/programmatic-email-api/send-email-api/attachments)
says that it is 2MB per attachment and 10MB in total, it seems like 2MB
is a soft limit and the 413 error only throws when the attachments
exceed 10MB in total.

![image](https://github.com/user-attachments/assets/23905adf-44b8-47d8-9b5f-7498f7d4da00)

## Solution

Check for the specific error code: `attachment_limit` and throw step
error for that.
<img width="895" alt="image"
src="https://github.com/user-attachments/assets/032a4a22-31b8-42a0-a0a9-77ce3a78b02f">

## Tests
- [x] Normal executions still work with postman
- [x] Attachments under the size of 10 MB works although it's slow
- [x] One attachment fails when it exceeds 10MB
- [x] Multiple attachment fails when they exceed 10MB
## Problem
There is no warning/prompt when user is deleting a connection.
Other delete actions on Apps/Pipes/Tiles have a warning/prompt when user
is attempting to delete.

Closes [PLU-361:
warning-when-deleting-connections](https://linear.app/ogp/issue/PLU-361/warning-when-deleting-connections)

## Solution
Add warning/prompt modal when user is attempting to delete a connection.


**Improvements**:
- Refactor Tiles to use MenuAlertDialog instead of inline AlertDialog
- Add toast on Tile delete success


## Before & After Screenshots

**BEFORE**:
-  No warning/modal, connection is deleted immediately.


https://github.com/user-attachments/assets/1d6ccd95-76bf-4e00-ad43-615902f3aa22

- No notification on Tile delete success

**AFTER**:
- Delete Connection


https://github.com/user-attachments/assets/c5b89692-26b1-47b3-80fe-20b64918f622

- Delete Tile


https://github.com/user-attachments/assets/c68e63df-9e30-4071-91f0-968f000524fe


## Tests
- [X] Modal appears when user attempts to delete a connection.
- [X] Connection is deleted successfully upon confirmation on modal.
- [X] Able to delete Tiles
- [X] Able to duplicate Pipes
- [X] Able to delete Pipes

**New scripts**:
- `packages/frontend/src/components/MenuAlertDialog/index.tsx` : generic
MenuAlertDialog for reuse across delete actions (updated from
`plumber/packages/frontend/src/components/FlowRow/MenuAlertDialog.tsx`)
@m0nggh m0nggh requested a review from a team as a code owner November 19, 2024 06:50
@datadog-opengovsg
Copy link

Datadog Report

Branch report: develop-v2
Commit report: 1460a33
Test service: plumber

✅ 0 Failed, 687 Passed, 0 Skipped, 2m 3.02s Total Time
➡️ Test Sessions change in coverage: 1 no change

@m0nggh m0nggh merged commit 54300e7 into production Nov 19, 2024
3 of 5 checks passed
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.

3 participants