-
Notifications
You must be signed in to change notification settings - Fork 51
Studio: Be more specific in push success email by listing individual sync elements #2007
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
Open
katinthehatsite
wants to merge
69
commits into
trunk
Choose a base branch
from
fix/send-specific-data-to-backend-for-email
base: trunk
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
69 commits
Select commit
Hold shift + click to select a range
ee2c1c2
Use local tree for push
05b23d9
Use error handling
6e9e558
Ensure specific selections are added
2d0dfac
Clean up empty line
63a1e77
Fix tests
36f17e1
Optimization
0472b72
Merge branch 'trunk' of github.com:Automattic/studio into fix/pull-fo…
8ada7ce
Remove unintended changes
a1b1837
Pre-fetch the whole tree
e542b70
Remove duplication
b67445f
Remove further exclusions
20b694a
Further improvements
fe41179
Merge branch 'trunk' of github.com:Automattic/studio into fix/pull-fo…
22c27ee
Improvements
6f9dca0
Add missing dependency
7d81151
Futher improvements
1a6843a
Resolve directories size error
2ebef8a
Cleanup
009ce4a
Merge trunk
4a72cb2
Refactor
3c19865
Clean up children and set expansion to true by default
970dbe1
db17cb2
Specifically define type order
98a5752
Remove constants with trailing slashes
ea1bd16
Fix path concatenation across platforms
369f7b8
Switch from null to array
963af9b
Clean up shouldExcludePathFromSync
a9cfee8
Simplify code and removed unused functions
30c8bc1
Minor cleanup
c428577
Rename function to RawDirectoryEntry
d90828c
Rename function to useTopLevelSyncTree
224bd8a
Remove an empty array
2fa32c6
Clean up ternary operator
df2721d
Clean up checked parameter
f4e8b0f
Adjust path for windows
747bece
Remove useMemo
783c7a3
Remove duplication
f020ae5
Cleanup code leftovers
e71ba1b
Solve issue with zip
6295c1c
Traverse the tree just once
d2a533e
Clean up repetitions
36608b7
Minor improvements
897ebb9
Cleanup
76fc204
Ensure that categories are constructed in a function
1ed6095
Improve function name
026167f
Fix export and minor improvements
18d80d3
Fix unintended changes
c226c66
Simplify some selections logic
66da094
Remove dead code
928e800
Further simplifications
7699c01
Merge branch 'trunk' of github.com:Automattic/studio into fix/pull-fo…
bfbd5fa
Fix selections errors in sync options
8a79c39
Fix tests on /convert-tree-to-options-to-sync.tsx
ca775b6
Fix exporter tests
b3db695
Simplify the function to specificSelectionPaths
0051c3f
Rename fileNode
6faa332
Removed unused type
e6170b7
Adjust git and node modules exclusion
85aff5d
Move tree props directly in
9a5ea61
Clean up local tree
5db98b7
Use descriptive names
0b3ad02
Ensure that we use curly brackets
64ecac1
Clean up trailing slashes
3fd8dce
Ensure that there is no side effects
d3ae5c4
Fix regex
4941d23
Pass to backend data for specific folders and files
8d282d5
Merge branch 'trunk' of github.com:Automattic/studio into fix/send-sp…
e2af41b
Cleanup
d0bc1cf
Adjust format
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@katinthehatsite , Is there any specific reason we are sending the option
paths?I think that option is not necessary. For the same reason I would suggest renaming
include_path_listvar name. Maybepush_selection_paths? 🤔 .There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we need to send SQL which does not come as a part of
wp-contentso that we can display information regarding the database being pushed to the user?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For example, this is what we get when choosing database:
And this is what we get when choosing all the files:
And this is what happens when we select just database:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My comment is specific about appending
pahtsto the options we send to the backend:const options = optionsToSync ? [ ...optionsToSync, 'paths' ] : [ 'paths' ];Studio was not using that option before, and AFAIK that option is not necessary.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was suspicious of the
pathsoption because the official parameterinclude_path_listthat VaultPress requires needs to be in base64. It seems to me that we are sending both thepathsoption and theinclude_path_listto the backend, and then from the backend to VaultPress. However, theinclude_path_listwill actually be ignored because it's not in base64.Yes, sqls is a valid option, that we need to send when database is checked.
Those are correct values ! 👌