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

Allow users to configure a personal OpenAI API key #1496

Merged
merged 13 commits into from
Aug 1, 2024

Conversation

Melhaya
Copy link
Contributor

@Melhaya Melhaya commented Jul 2, 2024

Having an API key per user will allow users to utilize the AI capabilities within CodeHarbor (i.e. Unittest generator)

  • The generate unittest button is now always visible
  • If key is invalid or missing, an alert is shown to the user
  • If key is available and valid, a unittest will be created for the task

@Melhaya Melhaya self-assigned this Jul 2, 2024
@Melhaya Melhaya marked this pull request as draft July 2, 2024 12:00
Copy link

codecov bot commented Jul 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.83%. Comparing base (df6988e) to head (dbc61fc).
Report is 7 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1496      +/-   ##
==========================================
+ Coverage   93.77%   93.83%   +0.06%     
==========================================
  Files         122      123       +1     
  Lines        2955     2985      +30     
==========================================
+ Hits         2771     2801      +30     
  Misses        184      184              

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

@Melhaya
Copy link
Contributor Author

Melhaya commented Jul 2, 2024

I managed to add an API key text area for users to input their key. When I edit the profile and add a key and update, its still blank. I am not really sure what I need to do to make it persist.

@MrSerth @kkoehn : With the current changes I made, am I on the right track to have each user have their own API key? Once I can persist the update in the profile, the next step will be to actually use that for the unit test generator.

I also noticed that the schema.db had also changed again. do I need to turn it back to how it was? I only added the line t.string "personal_api_key" but noticed that the code also worked without it. I can remove that line.

Update: I was able to persist the API key in the user profile by adjusting the RegistrationsController file. Current challenge is to use the API key given by a user in the unit test generator. Not sure which file to adjust for this. need guidance on how to proceed. Thanks

Screenshot 2024-07-02 at 14 35 15

@Melhaya Melhaya force-pushed the add_gpt_api_key_per_user branch 2 times, most recently from 8180348 to efe342b Compare July 2, 2024 17:06
@kkoehn
Copy link
Collaborator

kkoehn commented Jul 2, 2024

I managed to add an API key text area for users to input their key. When I edit the profile and add a key and update, its still blank. I am not really sure what I need to do to make it persist.

@MrSerth @kkoehn : With the current changes I made, am I on the right track to have each user have their own API key? Once I can persist the update in the profile, the next step will be to actually use that for the unit test generator.

I also noticed that the schema.db had also changed again. do I need to turn it back to how it was? I only added the line t.string "personal_api_key" but noticed that the code also worked without it. I can remove that line.

Update: I was able to persist the API key in the user profile by adjusting the RegistrationsController file. Current challenge is to use the API key given by a user in the unit test generator. Not sure which file to adjust for this. need guidance on how to proceed. Thanks

Good work on figuring out the strong parameters!

To use user-specific API-Keys you will have to touch multiple points:

  1. The task_policy should check for the user-key and not the "application-wide key"
  2. The "application-wide key" and all remnants should be removed
  3. Add the API-Key as parameter to your GptGenerateTests-Service
  4. In your generate_test-Action in the tasks_controller supply the API-Key to the service-call

I hope this gives you a way forward, if you need more help, let me know!

app/models/user.rb Outdated Show resolved Hide resolved
@Melhaya Melhaya force-pushed the add_gpt_api_key_per_user branch 2 times, most recently from a496065 to d3b9a89 Compare July 2, 2024 22:55
@Melhaya Melhaya requested review from MrSerth and kkoehn July 2, 2024 22:59
@Melhaya
Copy link
Contributor Author

Melhaya commented Jul 2, 2024

Everything should now work as intended. There is only one 'issue' that I can't seem to find a solution for.

In the code, I am raising Gpt::InvalidApiKeyError if the user's API key is empty or invalid. the alert message is shown correctly when the API key is invalid. but for some reason, I get a You are not authorized for this action. alert when the API key is empty.

I figured out the issue is in the task_policy.rb

Currently the task policy says the following:

  def generate_test?
    user.present? and user.personal_openai_api_key.present? and update?
  end

When removing and user.personal_openai_api_key.present? , everything works and alerts are shown correctly. But now I can't seem to figure out the task_policy_spec.rb. I think I should remove most of the contexts there since the action of generating unit tests is permitted to any user. the alerts and checks are done somewhere else. I have pushed these changes now for you to see. If it is correct, then this should be ready to merge maybe with some minor cleanups that you might mention. :)

@Melhaya Melhaya marked this pull request as ready for review July 3, 2024 11:03
Copy link
Collaborator

@kkoehn kkoehn left a comment

Choose a reason for hiding this comment

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

Thanks for the quick work with this PR. It looks good in general and seems to work fine, but I found some points that could be improved and had some questions.

app/views/tasks/show.html.slim Show resolved Hide resolved
db/schema.rb Show resolved Hide resolved
app/views/users/show.html.slim Outdated Show resolved Hide resolved
app/policies/task_policy.rb Outdated Show resolved Hide resolved
app/services/task_service/gpt_generate_tests.rb Outdated Show resolved Hide resolved
app/services/task_service/gpt_generate_tests.rb Outdated Show resolved Hide resolved
spec/controllers/tasks_controller_spec.rb Outdated Show resolved Hide resolved
spec/policies/task_policy_spec.rb Outdated Show resolved Hide resolved
app/views/users/registrations/edit.html.slim Outdated Show resolved Hide resolved
@Melhaya Melhaya force-pushed the add_gpt_api_key_per_user branch 2 times, most recently from 7748b49 to a57a16f Compare July 4, 2024 11:10
@Melhaya Melhaya requested a review from kkoehn July 4, 2024 20:57
@Melhaya Melhaya mentioned this pull request Jul 8, 2024
10 tasks
@Melhaya Melhaya force-pushed the add_gpt_api_key_per_user branch 3 times, most recently from 01fda2d to 3ceb522 Compare July 9, 2024 18:44
@Melhaya
Copy link
Contributor Author

Melhaya commented Jul 9, 2024

@kkoehn I don't understand why the tests are currently failing. what changed?

Could you also help me resolve the conflict, please? I have updated my branch but the conflict still exists.

app/models/user.rb Outdated Show resolved Hide resolved
app/services/task_service/gpt_generate_tests.rb Outdated Show resolved Hide resolved
app/views/tasks/show.html.slim Outdated Show resolved Hide resolved
app/views/users/registrations/edit.html.slim Outdated Show resolved Hide resolved
app/views/users/show.html.slim Outdated Show resolved Hide resolved
config/locales/de/external/devise.yml Outdated Show resolved Hide resolved
@Melhaya Melhaya force-pushed the add_gpt_api_key_per_user branch 2 times, most recently from dce3b5b to e38ea79 Compare July 10, 2024 22:17
@Melhaya Melhaya force-pushed the add_gpt_api_key_per_user branch 2 times, most recently from 54fee59 to e1e1d45 Compare July 11, 2024 12:54
Added tests in user.rb

remove redundant code in spec files
Copy link
Collaborator

@kkoehn kkoehn left a comment

Choose a reason for hiding this comment

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

All of my review points have been addressed. Good work!

app/models/user.rb Outdated Show resolved Hide resolved
@Melhaya Melhaya requested a review from MrSerth July 30, 2024 20:35
@Melhaya Melhaya requested a review from kkoehn July 31, 2024 08:48
@MrSerth MrSerth changed the title Adding an API Key per user Allow users to configure a personal OpenAI API key Aug 1, 2024
@MrSerth MrSerth merged commit 62ed5e3 into master Aug 1, 2024
13 checks passed
@MrSerth MrSerth deleted the add_gpt_api_key_per_user branch August 1, 2024 12:57
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.

3 participants