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

[ProgressV2] Create a new feature feedback endpoint for progress feedback #57990

Merged
merged 6 commits into from Apr 15, 2024

Conversation

lfryemason
Copy link
Contributor

@lfryemason lfryemason commented Apr 12, 2024

Warning!!

The AP CSP Create Performance Task is in progress. The most critical dates are from April 3 - April 30, 2024. Please consider any risk introduced by this PR that could affect our students taking AP CSP. Code.org students taking AP CSP primarily use App Lab for their Create Task, however a small percent use Game Lab. Carefully consider whether your change has any risk of alterering, changing, or breaking anything in these two labs. Even small changes, such as a different button color, are considered significant during this time period. Reach out to the Student Learning team or Curriculum team for more details.

Based on this PR to add LTI feedback: https://github.com/code-dot-org/code-dot-org/pull/57517/files

This adds a simple create and show feedback endpoint for a thumbs up or thumbs down feedback form. This feedback is specifically intended for new features and will be used for the new progress view feedback form.

This feedback is reusable because it uses a form_key identifier to determine which feedback form the user has answered. For the progress implementation, this will be progress_v2 which is currently the only allowed value.

Adds a migration to create the table and API endpoints. These endpoints will be called in a future PR.

Links

TEACH-1012

Testing story

Unit tests for controller

Deployment strategy

Follow-up work

Creating the FE component that will call the added endpoints.

Privacy

Security

Authenticating users calling the feedback form are teachers.

Caching

PR Checklist:

  • Tests provide adequate coverage
  • Privacy and Security impacts have been assessed
  • 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

@lfryemason lfryemason marked this pull request as ready for review April 12, 2024 15:27
@lfryemason lfryemason requested a review from a team as a code owner April 12, 2024 15:27
test 'show - returns nothing when no Feedback exists for the current user' do
get :show, params: {form_key: 'progress_v2'}, format: :json

assert_response :ok
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you check for an empty response body here?


t.index [:user_id, :form_key], unique: true

t.datetime :created_at, null: false
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a more standard way of creating created_at and updated_at columns.

Suggested change
t.datetime :created_at, null: false
t.timestamps

assert_response :unprocessable_entity
assert_equal '["Satisfied is not included in the list"]', response.body
end

Copy link
Contributor

Choose a reason for hiding this comment

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

A test to ensure users can't leave feedback on the same thing twice would be nice.

@lfryemason lfryemason merged commit 9a47018 into staging Apr 15, 2024
2 checks passed
@lfryemason lfryemason deleted the lfm/p-feedback-1 branch April 15, 2024 15:44
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.

None yet

2 participants