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: Add an api to edit onboarding extension request details before approval or rejection #2334

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

pankajjs
Copy link
Contributor

@pankajjs pankajjs commented Jan 3, 2025

Date: 3 Dec, 2025

Developer Name: @pankajjs


Issue Ticket Number

Description

  • This PR adds feature for onboarding user to edit the onboarding extension details before it is approved or rejected by super-users.
  • This PR contains only the changes needed for the feature.
  • There is a another PR for test.

Documentation Updated?

  • Yes
  • No

Under Feature Flag

  • Yes
  • No

Database Changes

  • Yes
  • No

Breaking Changes

  • Yes
  • No

Development Tested?

  • Yes
  • No

Screenshots

Screenshots Screenshot 2025-01-09 at 7 17 30 PM Screenshot 2025-01-09 at 7 17 40 PM Screenshot 2025-01-09 at 7 17 51 PM Screenshot 2025-01-09 at 7 18 10 PM Screenshot 2025-01-09 at 7 18 53 PM

Test Coverage

Test coverage has been added in test PR #2335

Additional Notes

routes/requests.ts Fixed Show fixed Hide fixed
@pankajjs pankajjs force-pushed the feat/add-api-to-edit-request branch from acb162a to 82d007e Compare January 7, 2025 13:33
@pankajjs pankajjs marked this pull request as ready for review January 7, 2025 13:44
routes/requests.ts Dismissed Show dismissed Hide dismissed
@pankajjs pankajjs force-pushed the feat/add-api-to-edit-request branch 4 times, most recently from 05611b1 to eb86437 Compare January 13, 2025 16:25
@pankajjs pankajjs force-pushed the feat/add-api-to-edit-request branch from eb86437 to 31c470b Compare January 15, 2025 07:22
* @returns {Promise<OnboardingExtensionResponse>} - Returns a promise that resolves to a response indicating success or failure.
*/
export const updateOnboardingExtensionRequestController = async (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find a validation check to ensure that this update request is being made either by the request owner or a superuser? Can you please tell me the line number?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyone who is authenticated can update the request same as our Task ER. We are storing logs to check who is updating the request. In some cases, we need that the request is updated by an authenticated user. If we constraint this feature to request owner, we can not deal with uncertain cases.
What do you think on this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know about the backend implementation of the task ER but the frontend logic is such that only owner of that requests or superuser can update ERs. This is a product question , I think we should ask @ankushdharkar about this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure

Copy link
Contributor Author

@pankajjs pankajjs Jan 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you are right. We should only allow the request owner or super user to edit the details.
Thanks for pointing it out.

- Add common validator to redirect request based on type of extension
- Add type field in onboarding extension validator
- Import addLog from services to make it available for stubbing while testing
- Moved response messages to constants file
- Reuse single instance of current date in request and log model for consistent data
@pankajjs pankajjs force-pushed the feat/add-api-to-edit-request branch from 31c470b to 70e79fb Compare January 15, 2025 07:54
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.

Feature : Extend Onboarding Override for Discord Server
2 participants