Skip to content

Add purge mailboxes for O365 Org #623

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.

Sign up for GitHub

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

Jump to bottom
Merged
ariquinones merged 1 commit into from
Oct 18, 2021
Merged

Add purge mailboxes for O365 Org #623

ariquinones merged 1 commit into from
Oct 18, 2021

Conversation

@Jchazari
Copy link
Contributor

@Jchazari Jchazari commented Oct 7, 2021 •
edited
Loading

@codecov-commenter
Copy link

codecov-commenter commented Oct 8, 2021 •
edited
Loading

Codecov Report

Merging #623 (6140a5b) into vbo-purge-beta (01a3489) will decrease coverage by 22.74%.
The diff coverage is 66.66%.

❗ Current head 6140a5b differs from pull request most recent head 9beb31c. Consider uploading reports for the commit 9beb31c to get more accurate results
Impacted file tree graph

@@                 Coverage Diff                 @@
##           vbo-purge-beta     #623       +/-   ##
===================================================
- Coverage           83.63%   60.88%   -22.75%     
===================================================
  Files                 711      726       +15     
  Lines                6368     7962     +1594     
  Branches              388      461       +73     
===================================================
- Hits                 5326     4848      -478     
- Misses               1030     3102     +2072     
  Partials               12       12
Impacted Files Coverage Δ
src/sdk/model/o365/o365-purge-mailbox.ts 0.00% <0.00%> (ø)
src/sdk/model/o365/o365-marked-purge-mailbox.ts 100.00% <100.00%> (ø)
src/sdk/model/o365/o365-organization.ts 100.00% <100.00%> (ø)
src/sdk/model/event/event-list.ts 0.00% <0.00%> (-100.00%) ⬇️
src/sdk/model/event/event-filter-params.ts 0.00% <0.00%> (-100.00%) ⬇️
src/sdk/model/push/push-channel.ts 0.00% <0.00%> (-88.68%) ⬇️
src/sdk/model/event/event.ts 0.00% <0.00%> (-83.34%) ⬇️
src/sdk/auth/direct-grant-auth-provider.ts 26.86% <0.00%> (-56.72%) ⬇️
src/sdk/service/http/http.ts 37.93% <0.00%> (-55.18%) ⬇️
src/sdk/model/vdc/vdc-summary.ts 51.51% <0.00%> (-48.49%) ⬇️
... and 34 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 01a3489...9beb31c. Read the comment docs.

@Jchazari Jchazari marked this pull request as ready for review October 8, 2021 15:34
iamtash
iamtash reviewed Oct 11, 2021
View reviewed changes
Copy link
Member

@iamtash iamtash left a comment

Choose a reason for hiding this comment

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

creating a O365MailboxPurgeRequest class would keep with a pattern we have for POST requests but not sure if it is compulsory here

@Jchazari
Copy link
Contributor Author

Jchazari commented Oct 15, 2021 •
edited
Loading

creating a O365MailboxPurgeRequest class would keep with a pattern we have for POST requests but not sure if it is compulsory here

@iamtash Yeah I also wanna say that pattern might not apply here since the body takes in an array instead of an object from the start. And creating a O365MailboxPurgeRequest would mean creating a JSON interface like: mailboxes: Array<Mailbox>; which is not even in the api docs

ariquinones
ariquinones suggested changes Oct 15, 2021
View reviewed changes
src/sdk/model/o365/o365-organization.ts Outdated
* @returns {Promise<unknown>}
*/
/* istanbul ignore next: autogenerated */
async purgeMailboxes(mailboxes: Array<O365PurgeMailboxJson>): Promise<unknown> {
Copy link
Contributor

@ariquinones ariquinones Oct 15, 2021

Choose a reason for hiding this comment

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

Lets not take in the json interface, rather the class

@iamtash
Copy link
Member

iamtash commented Oct 15, 2021 •
edited
Loading

creating a O365MailboxPurgeRequest class would keep with a pattern we have for POST requests but not sure if it is compulsory here

@iamtash Yeah I also wanna say that pattern might not apply here since the body takes in an array instead of an object from the start. And creating a O365MailboxPurgeRequest would mean creating a JSON interface like: mailboxes: Array<Mailbox>; which is not even in the api docs

an example of a method that takes an array of TS class instances is Catalog.updateMetadata()

@Jchazari
Copy link
Contributor Author

Jchazari commented Oct 18, 2021

creating a O365MailboxPurgeRequest class would keep with a pattern we have for POST requests but not sure if it is compulsory here

@iamtash Yeah I also wanna say that pattern might not apply here since the body takes in an array instead of an object from the start. And creating a O365MailboxPurgeRequest would mean creating a JSON interface like: mailboxes: Array<Mailbox>; which is not even in the api docs

an example of a method that takes an array of TS class instances is Catalog.updateMetadata()

Nice! Updated!

iamtash
iamtash reviewed Oct 18, 2021
View reviewed changes
@ariquinones ariquinones merged commit 94f27d1 into ilanddev:vbo-purge-beta Oct 18, 2021
@Jchazari Jchazari deleted the add-purge-mailboxes-for-o365-org branch October 18, 2021 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iamtash iamtash iamtash approved these changes

@fradinni fradinni Awaiting requested review from fradinni

@JoMas971 JoMas971 Awaiting requested review from JoMas971

+1 more reviewer

@ariquinones ariquinones ariquinones approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Jchazari @codecov-commenter @iamtash @ariquinones