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

Show workshop created_at date on workshop dashboard #32453

Merged
merged 2 commits into from
Dec 18, 2019

Conversation

islemaster
Copy link
Contributor

Megan mentioned that she needs to look up the created_at field for a workshop a handful of times a year. Rather than handle these requests manually, it seems reasonable to add this metadata in an understated style to the end of the workshop view.

Screenshot from 2019-12-17 14-31-16

Testing story

Unit tests for the update to the workshop serializer. Manually tested the client change. The only reason I'm not adding unit tests here is that there are no unit tests for the workshop view right now but I'm adding a bunch of them in #31990 and they'll cover this new code path.

Reviewer Checklist:

  • Tests provide adequate coverage
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

@islemaster
Copy link
Contributor Author

Test failure is ProxyHelperTest, known issue on staging earlier today. Going to rebase and try again.

Copy link
Contributor

@hacodeorg hacodeorg left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -405,6 +405,13 @@ class Api::V1::Pd::WorkshopsControllerTest < ::ActionController::TestCase
assert_response :forbidden
end

test 'created_at is included in a serialized workshop response' do
Copy link
Contributor

Choose a reason for hiding this comment

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

This attribute is getting special treatment :) If we have to do this more often, we can consolidate all serialization tests in a WorkshopSerializerTest class.

@islemaster islemaster merged commit 7c9412d into staging Dec 18, 2019
@islemaster islemaster deleted the workshop-created-at branch December 18, 2019 23:15
islemaster added a commit that referenced this pull request Dec 20, 2019
Restores behavior introduced in
#32453 and regressed
during a complicated rebase in
#31990.  Most of the
change was preserved, but the JSX that actually renders the new
component was accidentally dropped.

I mentioned in the previous PR that I wasn't adding a test for the UI
because I was adding so many tests in the other PR (that regressed
this!) and it would be covered.  Turns out I was wrong!  So now I'm
adding a unit test for the Workshop component that verifies that we show
the created_at date as expected.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants