Skip to content

feat/AB#74292 People picker#725

Open
TaiKamilla wants to merge 35 commits into2.x.xfrom
feat/AB#74292_ABC-Create-people-picker-question
Open

feat/AB#74292 People picker#725
TaiKamilla wants to merge 35 commits into2.x.xfrom
feat/AB#74292_ABC-Create-people-picker-question

Conversation

@TaiKamilla
Copy link
Contributor

@TaiKamilla TaiKamilla commented Sep 4, 2023 •
edited by Matthis-M-ReliefApps
Loading

Description

In this PR the backend responds to the query GetPeople and send back some mock data

Useful links

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Improvement (refactor or addition to existing functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Screenshots

image

Checklist:

( * == Mandatory )

  • * I have set myself as assignee of the pull request
  • * My code follows the style guidelines of this project
  • * Linting does not generate new warnings
  • * I have performed a self-review of my own code
  • * I have put the ticket for review, adding the oort-frontend team to the list of reviewers
  • * I have commented my code, particularly in hard-to-understand areas
  • * I have put JSDoc comment in all required places
  • * My changes generate no new warnings
  • * I have included screenshots describing my changes if relevant
  • * I have selected labels in the Pull Request, according to the changes with code brings
  • I have made corresponding changes to the documentation ( if required )
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@TaiKamilla TaiKamilla added the enhancement Existing feature label Sep 4, 2023
@TaiKamilla TaiKamilla requested a review from a team September 4, 2023 14:18
@TaiKamilla TaiKamilla self-assigned this Sep 4, 2023
@TaiKamilla TaiKamilla changed the title mock data feat/AB#74292_ABC-Create-people-picker-question Sep 4, 2023
@NathanHGit NathanHGit mentioned this pull request Sep 6, 2023
Open
16 tasks
@NathanHGit NathanHGit requested review from NathanHGit and removed request for a team September 6, 2023 08:44
NathanHGit
NathanHGit approved these changes Sep 6, 2023
View reviewed changes
Copy link
Contributor

@NathanHGit NathanHGit left a comment

Choose a reason for hiding this comment

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

Looks to work fine

@MwanPygmay MwanPygmay changed the base branch from 2.1.x to beta September 26, 2023 08:47
@MwanPygmay MwanPygmay self-requested a review September 26, 2023 08:52
MwanPygmay
MwanPygmay approved these changes Sep 27, 2023
View reviewed changes
Copy link
Contributor

@MwanPygmay MwanPygmay left a comment

Choose a reason for hiding this comment

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

  • Added support for people type
  • Added support for people metadata

@TaiKamilla TaiKamilla force-pushed the feat/AB#74292_ABC-Create-people-picker-question branch from f74545f to 6602426 Compare October 9, 2023 07:41
github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 20, 2024
View reviewed changes
src/routes/upload/index.ts
true,
true
);
return res.status(200).send(insertRecordsMessage);

Check failure

Code scanning / CodeQL

Reflected cross-site scripting

Cross-site scripting vulnerability due to a [user-provided value](1).
src/utils/files/resourceExporter.ts
method: 'POST',
headers: this.axiosHeaders(),
data: {
query: metaQuery,

Check failure

Code scanning / CodeQL

Database query built from user-controlled sources

This query string depends on a [user-provided value](1).
src/routes/email/index.ts
} catch (err) {
// Specific try / catch so we can know what the error is
logger.error(err.message, { stack: err.stack });
return res.status(500).send(err.message);

Check warning

Code scanning / CodeQL

Exception text reinterpreted as HTML

[Exception text](1) is reinterpreted as HTML without escaping meta-characters.
src/server/pullJobScheduler.ts
Comment on lines 416 to 419
const duplicateRecords = await RecordModel.find({
form: pullJob.convertTo,
$or: filters,
}).select(selectedFields);

Check failure

Code scanning / CodeQL

Database query built from user-controlled sources

This query object depends on a [user-provided value](1).
@NathanHGit NathanHGit changed the base branch from beta to 2.x.x March 20, 2024 13:06
Matthis-M-ReliefApps
Matthis-M-ReliefApps suggested changes Mar 29, 2024
View reviewed changes
Copy link
Contributor

@Matthis-M-ReliefApps Matthis-M-ReliefApps left a comment

Choose a reason for hiding this comment

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

Not a real review but please implement the filter as explained in latest comment on https://dev.azure.com/WHOHQ/EMSSAFE/_workitems/edit/74292

@NathanHGit
Copy link
Contributor

NathanHGit commented Mar 29, 2024

@Matthis-M-ReliefApps

It has been implemented in the frontend to maintain the route generic
ReliefApplications/ems-frontend#1792

AntoineRelief
AntoineRelief reviewed Apr 24, 2024
View reviewed changes
@AntoineRelief
Copy link
Collaborator

AntoineRelief commented Apr 25, 2024

@Matthis-M-ReliefApps @MwanPygmay

I got some Axios errors when trying the feature, did you get some when testing on your side also?

@MwanPygmay
Copy link
Contributor

MwanPygmay commented Apr 25, 2024

@AntoineRelief do you have the scope for common services?

@AntoineRelief
Copy link
Collaborator

AntoineRelief commented Apr 25, 2024 •
edited
Loading

@AntoineRelief do you have the scope for common services?

@MwanPygmay
Apart if it had changed, I think so yes

Doing the same request on Postman, I can get the data
I can't from the code thus

@Matthis-M-ReliefApps
Copy link
Contributor

Matthis-M-ReliefApps commented Apr 25, 2024

@AntoineRelief do you have the scope for common services?

@MwanPygmay Apart if it had changed, I think so yes

Doing the same request on Postman, I can get the data I can't from the code thus

I remember I also had some requests errors when I tried to start a review on it last week

Matthis-M-ReliefApps
Matthis-M-ReliefApps suggested changes May 16, 2024
View reviewed changes
Copy link
Contributor

@Matthis-M-ReliefApps Matthis-M-ReliefApps left a comment

Choose a reason for hiding this comment

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

Seems all good except for the hardcoded string

Matthis-M-ReliefApps
Matthis-M-ReliefApps approved these changes May 21, 2024
View reviewed changes
Copy link
Contributor

@Matthis-M-ReliefApps Matthis-M-ReliefApps left a comment

Choose a reason for hiding this comment

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

Perfection

@AntoineRelief AntoineRelief changed the title feat/AB#74292_ABC-Create-people-picker-question feat/AB#74292 People picker Jun 3, 2024
@AntoineRelief AntoineRelief marked this pull request as draft June 14, 2024 07:37
@MwanPygmay MwanPygmay marked this pull request as ready for review June 19, 2024 10:11
@AntoineRelief
Copy link
Collaborator

AntoineRelief commented Jun 26, 2024 •
edited
Loading

@Matthis-M-ReliefApps @MwanPygmay

looking at it again, I can see it's not yet complete:

  • history is not working fine
  • filtering in aggregations & layouts is not proposing any filter operator
  • export from resource is not working ( not the one from the grid, but from the resource page )

I can't merge it like that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AntoineRelief AntoineRelief AntoineRelief left review comments

@MwanPygmay MwanPygmay Awaiting requested review from MwanPygmay

+2 more reviewers

@NathanHGit NathanHGit NathanHGit approved these changes

@Matthis-M-ReliefApps Matthis-M-ReliefApps Matthis-M-ReliefApps approved these changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

@TaiKamilla TaiKamilla

@NathanHGit NathanHGit

Labels

enhancement Existing feature

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@TaiKamilla @NathanHGit @AntoineRelief @MwanPygmay @Matthis-M-ReliefApps