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

Add English variants to select2_local directory #3895

Merged
merged 7 commits into from
Feb 25, 2021

Conversation

michaelmichael
Copy link
Contributor

Description
Fixes #3877
This PR adds dummy en-[XX].js files to the select2 locales directory in the admin to fix an issue where Rails returns an asset pipeline error when the user selects English (GB), English (New Zealand), English (Australia), or English (IN).

One note: This PR seems to cause this spec to fail intermittently. Apparently Capybara can't find the .flash element sometimes. Other times it works fine.

However, I noticed a comment in the spec that line 35 is only present to prevent the spec from failing due to an issue with select2. What's odd is that if I comment out line 35, the spec (with my changes introduced), always passes.

I can remove that line from the spec since it doesn't appear to serve any purpose other than to act as a talisman against failure, but I wanted to present it here first.

Checklist:

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have updated Guides and README accordingly to this change (if needed)
  • I have added tests to cover this change (if needed)
  • I have attached screenshots to this PR for visual changes (if needed)

Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

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

Thanks @michaelmichael! ❤️

I'm fine removing that line, it was probably related to some bug with selenium that has been now fixed. If it will cause the flaky to happen again, we can easily revert the commit and reintroduce it. Let me link #3278 for reference.
Thanks for spotting it!

@michaelmichael
Copy link
Contributor Author

@kennyadsl Ok, I'll remove it. Cleaner to do it in this PR or another?

@kennyadsl
Copy link
Member

I'd say let's try to remove that code in this PR and re-run the CI a couple of times to see if the flaky goes away. If it's related to this PR somehow, it's better if we merge them together.

@michaelmichael
Copy link
Contributor Author

Ok, I squashed those commits.

@kennyadsl
Copy link
Member

@michaelmichael the Rails 5.2 failure seems real, I tried to re-run the CI and it happened again. Re-running it again to see what happens.

@kennyadsl
Copy link
Member

@michaelmichael apparently, removing that line is reintroducing the flaky spec we were having before adding it. Can you please remove the commit that removes that line? I'd like to see the intermittent specs you were talking about failing in the CI.

@kennyadsl
Copy link
Member

@michaelmichael Thanks, I'm going to run specs on the CI several times to see if we have that flaky spec yet and will let you know, thanks!

@michaelmichael
Copy link
Contributor Author

Sounds good. FWIW, since I merged in the upstream changes I stopped getting the flaky spec in my local environment. Maybe it was something in my dev environment all along 🤷🏽

@kennyadsl kennyadsl merged commit 90d739b into solidusio:master Feb 25, 2021
kennyadsl pushed a commit that referenced this pull request Feb 25, 2021
* Add English variants to select2_local directory

Co-authored-by: Michael Caviness <[email protected]>
liftforward-ci-zz pushed a commit to liftforward/solidus that referenced this pull request Dec 14, 2021
rmparr pushed a commit to rmparr/solidus that referenced this pull request Jun 1, 2022
rmparr pushed a commit to rmparr/solidus that referenced this pull request Jun 1, 2022
cpfergus1 pushed a commit to cpfergus1/solidus that referenced this pull request Aug 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The asset "solidus_admin/select2_locales/select2_locale_en-GB.js" is not present in the asset pipeline
3 participants