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

🧹 Comment out check for reports #2116

Closed
wants to merge 230 commits into from
Closed

Conversation

jeremyf
Copy link
Contributor

@jeremyf jeremyf commented Dec 22, 2023

For this spec, if Hyrax.config.analytics? is enabled we need a legato
user to query the analytics. Which creates a weird dependency that we
may not want in this integration test.

(This stuff is best tested in view tests; but here we are)

I opted to not remove the spec but instead favor the
Hyrax.config.analytics? which, if it is true likely means we have ENV
vars that are in play.

orangewolf and others added 30 commits September 20, 2023 23:59
jeremyf and others added 25 commits December 21, 2023 11:49
…mponent

🐛 Adding document_component to blacklight's config
Hyrax has :clean_repo and Hyku has :clean

Sometimes folks copy over specs from Hyrax, and bring along the
:clean_repo; which looks like it should work.

With this commit, we bring that logic along!
Perhaps it's flappy; but let's see.
🧹 Allow :clean or :clean_repo to work for the cleaners
🧹 Fix button class for Bootstrap 3 to 4
Rubocop seams to prefer the Layout namespace for LineLength.
Checking the HTML (on the CircleCI SSH environment), it appears that the
selector should work in test.  But it is not.  So I'm removing the specificity.
…deprecation

🧹 Favor Layout/LineLength over Metrics/LineLength
…ng-spec

🧹 The selector is not working
Prior to this commit, we did not automatically clean the features.  The
below ripgrep (and output) shows that there were some features which did
not start from a clean state.

```
rg "(clean|clean_repo):" spec/features --files-without-match
``

```
spec/features/accounts_spec.rb
spec/features/proprietor_spec.rb
spec/features/featured_collections_spec.rb
spec/features/user_roles_spec.rb
spec/features/oai_pmh_spec.rb
```
…specs-run-clean

🧹 Ensure feature specs run clean
This commit will reconcile the services that are overrides for Hyrax
with the Hyrax 5.0.0rc2 version.  There were a number of overrides that
were changed to the decorator pattern.
* adventist_dev:
  Appease rubocop
  clean up from pr review
This is not tested in the UI, but the
`show_works_or_works_that_contain_files` remains after the troublesome
advanced query filters.
This commit addresses comments from the review but one thing that is of
note is loading the I18n translations in the application.rb file.  We
needed this because our decorators load prior to I18n loads the locales
in our config/locales directory for them to use so we were getting
missing translations.
…hing

🧹 Attempting to find and squash bug
For this spec, if `Hyrax.config.analytics?` is enabled we need a legato
user to query the analytics.  Which creates a weird dependency that we
may not want in this integration test.

(This stuff is best tested in view tests; but here we are)

I opted to not remove the spec but instead favor the
`Hyrax.config.analytics?` which, if it is true likely means we have ENV
vars that are in play.
@jeremyf jeremyf added the patch-ver for release notes label Dec 22, 2023
@ShanaLMoore
Copy link
Collaborator

Hey Jeremy,

This one may be worth discussing.

Kirk and I addressed the failing spec in another way. We had noticed that our local locale file was not taking precedence over the hyrax gem's version. Correcting that issue makes the spec pass. So is adding this conditional still worth while?

Base automatically changed from hyrax-5-upgrade to main January 5, 2024 18:12
@jeremyf jeremyf closed this Feb 6, 2024
@jeremyf jeremyf deleted the hyrax-5-upgrade-restore-specs branch February 6, 2024 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch-ver for release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants