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

Adjust visuals sync progress #2448

Merged
merged 74 commits into from
Dec 5, 2021
Merged

Conversation

Rahmon
Copy link
Contributor

@Rahmon Rahmon commented Nov 9, 2021

Description of the Change

Alternate Designs

Benefits

Possible Drawbacks

Verification Process

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests passed.

Applicable Issues

Changelog Entry

@Rahmon Rahmon self-assigned this Nov 9, 2021
@mckdemps mckdemps added this to the 4.0.0 milestone Nov 9, 2021
@Rahmon Rahmon assigned felipeelia and unassigned Rahmon Nov 30, 2021
Copy link
Member

@felipeelia felipeelia left a comment

Choose a reason for hiding this comment

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

Some issues I found during my tests:

General

  • Progress bar doesn't seem to be working. As soon as I click on Sync Now, it goes directly to 100% but if I click on Show log I can see things happening
  • If the user pauses the sync and reload the page, the pause/stop should be visible. Clicking on Sync Now will resume it but it is not clear it works like that.

Errors count

In a site with content imported from WooCommerce sample data and tests/wpa/test-docs/content-example.xml, and also wit a script to cause the Limit of total fields [5000] in index [...] has been exceeded error

  • 350 posts per page causes a "Request Entity Too Large" error. That is outputted in the Errors tab but the counter isn't updated
  • Full log isn't "full". Errors should be outputted in both places, or we need to change the first tab name
  • Reducing the 350 to 100 I can see 24 lines of errors in that tab but the count says 123

includes/partials/header.php Outdated Show resolved Hide resolved
@@ -1,5 +1,306 @@
#ep-sync-output {
.elasticpress_page_elasticpress-sync .button:disabled {
Copy link
Member

Choose a reason for hiding this comment

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

Not a blocker for Beta 1, but we need to get back to this file before the official release and make use of some vars, especially for breakpoints. dashboard.css makes use of that and it increases readability a lot.

assets/js/sync.js Outdated Show resolved Hide resolved
'sync_stack' => [],
'network_alias' => [],
'start_time' => microtime( true ),
'start_date_time' => $start_date_time ? $start_date_time->format( 'D, F d, Y H:i' ) : false,
Copy link
Member

Choose a reason for hiding this comment

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

Not a blocker for Beta 1 but we should have this stored in another format, so people can translate that format later.

includes/partials/sync-page.php Outdated Show resolved Hide resolved
includes/partials/sync-page.php Outdated Show resolved Hide resolved
includes/partials/sync-page.php Outdated Show resolved Hide resolved
@felipeelia felipeelia assigned Rahmon and unassigned felipeelia Dec 2, 2021
@felipeelia felipeelia merged commit 80aa939 into 4.x.x Dec 5, 2021
@felipeelia felipeelia deleted the chore/adjust-visuals-sync-progress branch December 5, 2021 22:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants