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

Fix erroneous media search #593

Merged
merged 11 commits into from
Feb 16, 2024
Merged

Fix erroneous media search #593

merged 11 commits into from
Feb 16, 2024

Conversation

fosterfarrell9
Copy link
Collaborator

This PR fixes some strange behaviour of the media search. The user input of tags was in many cases not actually added to the SOLR search request. Also, there was some strange second search taking place if the tag operator was 'OR' and some text was given to search. The new behavior is way more consistent: The different search options (type, associated to, tags,...) are now
combined by 'AND' and the tag operator behaves according to the chosen operator OR/AND.

Splines and others added 6 commits January 11, 2024 19:05
* Reapply first fix for Reader/Media

See discussion on #574 for further details.
Previous PR for this was #576, closed in favor of this one
as this directly branches off the new "dev" branch.

* Correctly show latest post (might be current_user's comment)

* Fix update of unread comments logic in comments controller

* Fix update icon logic and latest post comment

* Simplify latest comment logic

* Improve code comments

* Further improve comments

* Fix wording in comment

* Fix construction of media array & use `.blank?` instead of `.empty?`
* Add dummy migration

* Implement migration for unread comment flag

* Remove unnecessary comment

* Declare migration as not idempotent

* Use array.length instead of counting

* Throw error to prevent revert of migration

* Fix severe flaws in unread comments migration

* Simplify Reader retrieval

* Use the more explicit `.nil?` method

* Update migration date

* Fix annoying bug: don't use `.select!` but `.select`

* Polish migration

e.g. update comment, more suitable name for the method etc.

* Rename method according to #585
This is to reflect the corresponding renaming of the route
due to rubocop.
@Splines Splines changed the title Fix erratic media search Fix erroneous media search Feb 15, 2024
app/models/medium.rb Show resolved Hide resolved
(inside the media search, right now only frontend change)
This is such that we can still click on the label to select
the respective radio button, instead of only being able
to click on the radio button itself.
If the "all tags" option is enabled in the search, we should allow the
results to include *any* tag. This is automatically the case if we just
don't add any restriction regarding the tag ids in the SOLR search.

This change accompanies the frontend change to disable the AND/OR
buttons if the "all" button is selected meaning the user wants to search
for all tags. See #593 for more details.
Splines
Splines previously approved these changes Feb 15, 2024
Copy link
Member

@Splines Splines left a comment

Choose a reason for hiding this comment

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

Please use "Squash and merge"

@Splines Splines changed the base branch from dev to main February 16, 2024 00:01
@Splines Splines dismissed their stale review February 16, 2024 00:01

The base branch was changed.

@Splines Splines changed the base branch from main to dev February 16, 2024 00:01
@Splines Splines marked this pull request as draft February 16, 2024 00:02
@Splines Splines marked this pull request as ready for review February 16, 2024 00:02
Copy link
Member

@Splines Splines left a comment

Choose a reason for hiding this comment

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

Note I temporarily switched the base of this branch and then went ahead marking this PR as draft to remark it as ready for review to trigger our linting checks again. Don't know why the tests weren't executed, as they should be on every commit. Maybe a small hickup on GitHub's side who knows. Now the tests ran through ;)

Please use "Squash and merge"

@fosterfarrell9 fosterfarrell9 merged commit 95168d8 into dev Feb 16, 2024
3 checks passed
@fosterfarrell9 fosterfarrell9 deleted the fix/erratic-media-search branch February 16, 2024 10:48
Splines added a commit that referenced this pull request May 1, 2024
These changes are necessary due to #593.
Splines added a commit that referenced this pull request May 13, 2024
* Add Python script to run Rspec tests in docker container

This builds on the Ruby Test Explorer VSCode extension.
See my comment here: connorshea/vscode-ruby-test-adapter#30 (comment)

* Add Ruby Test explorer settings to `settings.json`

* Rename `run_cypress_tests` to `test` & use buildx

buildx is used for faster builds
we also set up ghcr to use docker layer caching for hopefully even
faster build times

* Fix paths to files and rename to `Testing`

* Use lowercase github org name

* Use GitHub Rspec formatter

* Clean up tests.yml & add comment with useful links

* Delete unnecessary dotfiles

* Fix path due to renaming of `run_cypress_tests` to `test`

* Add more comments to tests.yml

* Don't deal with cypress_runner here

* Only cache real build targets

Other layers will get cached anyways as they are external images.

* Source dummy docker env variables

* Add missing `require "rails_helper" statement

* Move comment to correct place in test

* Fix missing `=` in workflow file

* Source docker-dummy.env in docker test environment

* Don't source dummy docker env & add missing env variables

We just add the missing env variables to the docker-compose.yml config
for the test setup.

* Disable "can destroy users" test

* Copy missing app folder in Dockerfile (test setup)

* Remove creation of test database in workflow

* Precompile webpacker assets into docker test image

* Fix failing media search tests (first iteration)

* Fix media search tests

These changes are necessary due to #593.

* Upgrade Node.js dev dependencies

* Fix wrong ESLint semicolon-key value

* Remove pending test

* Freeze string assigned to constant (rubocop)

* Try to use `delete_all` instead of `destroy_all`

This may be a fix for the issue `Can't modify frozen hash` we get in the
pipeline during the unit tests.

* Don't delete Medium table as DatabaseClenaer takes care of it

* Improve database cleaner config according to docs

* Only rely on active record database cleaner gem

See the gem setup here:
https://www.rubydoc.info/github/DatabaseCleaner/database_cleaner#gem-setup

* Use before() & after() instead of around() in DB cleaner

* Explicitly require database_cleaner gem

* Use append_after() instead of append()

See here: https://www.rubydoc.info/github/DatabaseCleaner/database_cleaner#rspec-with-capybara-example

* Change directory to test folder

* Revert "Change directory to test folder"

This reverts commit ebbe53d.

* Try to use transactional fixtures

* Use database cleaner, omit before :all (instead use before :each)

* Pass token to Codecov test workflow

* Ignore test files in Codecov report

* Use testing GitHub environment for CI/CD
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.

2 participants