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 QuestionComponent #1404

Merged
merged 11 commits into from
Oct 22, 2023
Merged

Add QuestionComponent #1404

merged 11 commits into from
Oct 22, 2023

Conversation

pixeldesu
Copy link
Member

Requires #1403 to be merged first.

De-duplicates question displays across the application.

Note: Does not include any tests because...

  1. ApplicationComponent is just a base component exposing Devise user helpers to the components
  2. The above is making it exceptionally hard to unit test components that require user context, because Devise in the end requires Warden/controller context in tests and that's not a given.

I'll try to find a solution for the testing problem later.

@pixeldesu pixeldesu requested review from nilsding and raccube October 21, 2023 00:15
@codecov
Copy link

codecov bot commented Oct 21, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (2432010) 95.23% compared to head (3609c91) 95.27%.
Report is 16 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1404      +/-   ##
==========================================
+ Coverage   95.23%   95.27%   +0.03%     
==========================================
  Files         169      171       +2     
  Lines        2646     2664      +18     
==========================================
+ Hits         2520     2538      +18     
  Misses        126      126              
Files Coverage Δ
app/components/application_component.rb 100.00% <100.00%> (ø)
app/components/question_component.rb 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -0,0 +1,8 @@
en:
answers:
zero: "0 answers"
Copy link
Contributor

Choose a reason for hiding this comment

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

Could put something like "No answers" or "Unanswered" here.

Copy link
Member

Choose a reason for hiding this comment

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

Zarro answoors found

Copy link
Member Author

Choose a reason for hiding this comment

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

This technically doesn't appear anyway (answer count must be positive to show uppp), it's just added to match all numerical translation values

Copy link
Member

Choose a reason for hiding this comment

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

then i would suggest to change it to something more fun

Suggested change
zero: "0 answers"
zero: "zefix, do is jo nix"

@pixeldesu pixeldesu merged commit b5347de into main Oct 22, 2023
11 of 12 checks passed
@pixeldesu pixeldesu deleted the feature/question-component branch October 22, 2023 19:04
@sentry-io
Copy link

sentry-io bot commented Oct 30, 2023

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ ActionView::Template::Error: No route matches {:action=>"show", :controller=>"user", :username=>nil}, missing required keys: [... InboxController#show View Issue

Did you find this useful? React with a 👍 or 👎

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.

3 participants