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

relocate list and pages de-serialization functions #4852

Merged
merged 3 commits into from
Dec 15, 2023

Conversation

HiteshRepo
Copy link
Contributor

relocate list and pages de-serialization functions

Does this PR need a docs update or release note?

  • β›” No

Type of change

  • 🧹 Tech Debt/Cleanup

Issue(s)

#4754

Test Plan

  • πŸ’ͺ Manual
  • ⚑ Unit test
  • πŸ’š E2E

@HiteshRepo HiteshRepo added restore sharepoint lists category of sharepoint service labels Dec 14, 2023
@HiteshRepo HiteshRepo self-assigned this Dec 14, 2023
Copy link
Contributor

aviator-app bot commented Dec 14, 2023

Current Aviator status

Aviator will automatically update this comment as the status of the PR changes.
Comment /aviator refresh to force Aviator to re-examine your PR (or learn about other /aviator commands).

This PR was merged manually (without Aviator). Merging manually can negatively impact the performance of the queue. Consider using Aviator next time.


See the real-time status of this PR on the Aviator webapp.

Use the Aviator Chrome Extension to see the status of your PR within GitHub.

@ryanfkeepers
Copy link
Contributor

@HiteshRepo I assume this is only code movement, and contains no changes to the code itself. But it's difficult to know for sure, because the number of line additions and deletions is substantially different: 329 lines added, 283 removed. Can you please confirm whether you made any changes as part of moving the code?

What's helpful for PRs like this is if you can write something in the description to state whether reviewers need to look for additional changes. Something like: "No changes to logic in this PR; I only moved/renamed code." That'll give us extra confidence that the changes are simple and we don't need to investigate every line that was moved for comparison.

@HiteshRepo
Copy link
Contributor Author

e this is only code movement, and contains no changes to the code itself. But it's difficult to know for sure, because the number of line additions and deletions is substantially different: 329 lines added, 283 removed. Can you please confirm whether you made any changes as part of moving the code?

There is no code changes @ryanfkeepers in this PR. I separated the code changes in subsequent PRs branching out for this branch.

But here is the breakup:

  1. 187 lines removed from api/serialization.go
    • 171 lines added to api/lists.go
    • 13 lines added to api/pages.go
  2. 102 lines removed from api/serialization_test.go
    • 50 lines added to api/lists_test.go
    • 72 lines added to api/pages_test.go
    • 20 extra lines added because of setting up suite individually for pages and list
  3. rest are just renaming of functions from 'CreatePageFromBytes' to 'BytesToSitePageable' & 'CreateListFromBytes' to 'BytesToListable'

I think the 20-23 extra lines that got added that gets doubled and makes it a 46 gap b/w added and removed.

@ryanfkeepers
Copy link
Contributor

thank you for the explanation!

- adds methods to Lists client wrapper to create list and list items
- adds methods to Lists client wrapper to delete list
- handles graph error: General exception while processing by column
validation nil check
- handles graph error: Unable to identify column definition
- handles location type of columns and its data

#### Does this PR need a docs update or release note?
- [x] β›” No

#### Type of change

<!--- Please check the type of change your PR introduces: --->
- [x] 🌻 Feature

#### Issue(s)
#4754 

#### Test Plan

<!-- How will this be tested prior to merging.-->
- [x] πŸ’ͺ Manual
- [x] ⚑ Unit test
- [x] πŸ’š E2E
@HiteshRepo HiteshRepo merged commit dbf10d1 into 4754-backup-1 Dec 15, 2023
5 of 7 checks passed
@HiteshRepo HiteshRepo deleted the 4754-restore-3 branch December 15, 2023 21:47
Copy link

sonarcloud bot commented Dec 15, 2023

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

1 New issue
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

HiteshRepo added a commit that referenced this pull request Dec 21, 2023
relocate list and pages de-serialization functions

#### Does this PR need a docs update or release note?
- [x] β›” No

#### Type of change
- [x] 🧹 Tech Debt/Cleanup

#### Issue(s)
#4754 

#### Test Plan

<!-- How will this be tested prior to merging.-->
- [x] πŸ’ͺ Manual
- [x] ⚑ Unit test
- [x] πŸ’š E2E
HiteshRepo added a commit that referenced this pull request Dec 21, 2023
relocate list and pages de-serialization functions

#### Does this PR need a docs update or release note?
- [x] β›” No

#### Type of change
- [x] 🧹 Tech Debt/Cleanup

#### Issue(s)
#4754 

#### Test Plan

<!-- How will this be tested prior to merging.-->
- [x] πŸ’ͺ Manual
- [x] ⚑ Unit test
- [x] πŸ’š E2E
aviator-app bot pushed a commit that referenced this pull request Dec 22, 2023
adds list client apis and pagers to fetch, create and delete lists and list relations.
Changes previously approved in PRs:
- #4785
- #4815
- #4852

#### Does this PR need a docs update or release note?
- [x] β›” No

#### Type of change

<!--- Please check the type of change your PR introduces: --->
- [x] 🌻 Feature

#### Issue(s)
#4754 

#### Test Plan

<!-- How will this be tested prior to merging.-->
- [x] πŸ’ͺ Manual
- [x] ⚑ Unit test
- [x] πŸ’š E2E
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lists category of sharepoint service restore sharepoint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants