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

Closes #5640: fix translatePress compatibility #6219

Closed
wants to merge 2 commits into from

Conversation

Miraeld
Copy link
Contributor

@Miraeld Miraeld commented Oct 12, 2023

Description

I've done some refactor to improve the code. And also I've implemented the fixes requested.

Fixes #5640

Type of change

  • Enhancement (non-breaking change which improves an existing functionality).

Is the solution different from the one proposed during the grooming?

No

Checklists

Generic development checklist

  • My code follows the style guidelines of this project, with adapted comments and without new warnings.
  • I have added unit and integration tests that prove my fix is effective or that my feature works.
  • The CI passes locally with my changes (including unit tests, integration tests, linter).
  • Any dependent changes have been merged and published in downstream modules.
  • If applicable, I have made corresponding changes to the documentation. Provide a link to the documentation.

Test summary

  • I triggered all changed lines of code at least once without new errors/warnings/notices.
  • I validated all Acceptance Criteria of the related issues. (If applicable, provide proof).
  • I validated all test plan the QA Review asked me to.

@Miraeld Miraeld added type: enhancement Improvements that slightly enhance existing functionality and are fast to implement 3rd party compatibility Issues related to 3rd party compatibility like theme, plugin or hosting module: remove unused css labels Oct 12, 2023
@Miraeld Miraeld requested a review from a team October 12, 2023 00:31
@Miraeld Miraeld self-assigned this Oct 12, 2023
@Miraeld Miraeld force-pushed the enhancement/5640-translatePress-fix branch from 0dd0fc2 to c7327fb Compare October 12, 2023 00:42
@Miraeld Miraeld changed the title Enhancement/5640 translate press fix Closes #5640: fix translatePress compatibility Oct 12, 2023
@Mai-Saad Mai-Saad self-requested a review October 16, 2023 08:06
@Mai-Saad
Copy link
Contributor

Mai-Saad commented Oct 16, 2023

@Miraeld Thanks for the PR. Can still see the 400 error for home/fr on https://new.rocketlabsqa.ovh/ while permalinks with/without / . Can you please check 🙏
Screenshot from 2023-10-16 11-46-49

@Miraeld Miraeld force-pushed the enhancement/5640-translatePress-fix branch from c7327fb to 168c9ef Compare October 16, 2023 14:57
@CrochetFeve0251
Copy link
Contributor

@MathieuLamiot for it looks like the code is not working on the call we are fetching the response from the request.
My solution for this would be to add the status from the page (homepage or other ) inside the wp_wpr_rucss_used_css table.

Then we would just have to fetch that value from the table to know which queue we need to fetch the result from without having a bad JSON issue.

@MathieuLamiot
Copy link
Contributor

MathieuLamiot commented Oct 19, 2023

@CrochetFeve0251 Sounds like a good approach to me 👍
Let's compute the is_home at submission time and store it for later use.

@Mai-Saad
Copy link
Contributor

Closing this one in favour of #6266

@Mai-Saad Mai-Saad closed this Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3rd party compatibility Issues related to 3rd party compatibility like theme, plugin or hosting module: remove unused css type: enhancement Improvements that slightly enhance existing functionality and are fast to implement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RUCSS wrong home_url detection with TranslatePress enabled
4 participants