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

Authors bugfixes #374

Merged
merged 3 commits into from
Jun 12, 2024
Merged

Authors bugfixes #374

merged 3 commits into from
Jun 12, 2024

Conversation

damisul
Copy link
Collaborator

@damisul damisul commented Jun 12, 2024

Fixed several bugs introduced during Authorities refactoring:

  • fixed filtering by person's traits (gender, perdiod, birth and death years)
  • updated authors#to_manual_toc, #create_toc and #destroy actions to work with Authorities instead of People. There are many changes in #to_manual_toc, but they should not affect logic, I did them to satisfy linters

P.S. also please see #373

damisul added 2 commits June 12, 2024 12:36
- modified create_toc, to_manual_toc and destroy actions in authors_controller to use Authority instead of Person
- updated authors browse filter to properly filter by person's traits (period, gender, birth and death years)
@@ -1109,6 +1109,7 @@ he:
no_such_item: אין פריט כזה.
item: פריט
created_toc: נוצר דף יוצר עם תבנית ריקה
already_has_toc: Author already has TOC
Copy link
Collaborator Author

@damisul damisul Jun 12, 2024

Choose a reason for hiding this comment

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

When I wrote spec for #to_manual_toc it turned out we miss this string

@@ -282,81 +282,63 @@ def by_tag
end

def create_toc
author = Person.find(params[:id])
unless author.nil?
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually we have many places like this all over the code. They are useless, as ActiveRecord.find method raises an RecordNotFound exception if record with given id is not found. So we never get author.nil? condition satisfied, but code expects nil to be returned in such case.

@abartov abartov merged commit 36234af into master Jun 12, 2024
4 checks passed
@abartov abartov deleted the authors_bugfixes branch June 12, 2024 09:27
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