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 links attribute and gem setup fixes #105

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sosolidkk
Copy link

@sosolidkk sosolidkk commented Aug 14, 2024

What is the current behavior?

  1. Unable to run gem specs after the initial bundle install.
  2. Specs fail after executing bundle exec rake spec.
  3. Missing links attributes as per JSON:API Specification.

What is the new behavior?

1. Solved an issue with Rails secret_key_base config

I couldn't find any mention of secrets or secrets.yml in the project, and it also doesn't have an encrypted secrets file (credentials, secrets.yml.enc), any .env file or a in instruction on README to add the expect ENV var. Because of this, every time I tried to run the project, I encountered the following error:

An error occurred while loading spec_helper.
Failure/Error: secrets.secret_key_base = '_'

NameError:
  undefined local variable or method `secrets' for class Dummy
# ./spec/dummy.rb:87:in `<class:Dummy>'
# ./spec/dummy.rb:86:in `<top (required)>'
# ./spec/spec_helper.rb:10:in `<top (required)>'
No examples found.
No examples found.


Finished in 0.00003 seconds (files took 1.09 seconds to load)
0 examples, 0 failures, 1 error occurred outside of examples

Finished in 0.00003 seconds (files took 1.09 seconds to load)
0 examples, 0 failures, 1 error occurred outside of examples

To address this, I updated the configuration to match what the Rails::Application object expects. Now, the specs and gem setup work correctly, allowing the tests and QA rake tasks to run as expected without any additional changes.

2. Resolved an Issue with the 422 Error Description

The Rack utils.rb defines the HTTP 422 error as Unprocessable Content, but the specs expect it to be Unprocessable Entity. I'm not entirely sure which is correct, but according to the specification, it should be Unprocessable Content. In any case, I've updated the specs to accept both.

3. Added links Attribute in ErrorSerializer

I implemented a monkey patch in my app to provide this attribute, but I believe it should be included by default, similar to how the code attribute was previously added. Here's the specification reference for this attribute:

Checklist

Please make sure the following requirements are complete:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes /
    features)
  • All automated checks pass (CI/CD)

@sosolidkk
Copy link
Author

I didn't address on this PR, but there's seems to be a error being improperly raised on this spec.

with an exception
E, [2024-08-14T11:13:38.439786 #77087] ERROR -- : undefined method `raise_error!' for an instance of NotesController (NoMethodError)
/home/jpechaves/Github/jsonapi.rb/spec/dummy.rb:148:in `update'
/home/jpechaves/.asdf/installs/ruby/3.3.4/lib/ruby/gems/3.3.0/gems/actionpack-7.2.0/lib/action_controller/metal/basic_implicit_render.rb:8:in `send_action'
/home/jpechaves/.asdf/installs/ruby/3.3.4/lib/ruby/gems/3.3.0/gems/actionpack-7.2.0/lib/abstract_controller/base.rb:226:in `process_action'
/home/jpechaves/.asdf/installs/ruby/3.3.4/lib/ruby/gems/3.3.0/gems/actionpack-7.2.0/lib/action_controller/metal/rendering.rb:193:in `process_action'
# ...
      is expected to contain exactly {"code"=>nil, "detail"=>nil, "links"=>nil, "source"=>nil, "status"=>"500", "title"=>"Internal Server Error"}

@stas
Copy link
Owner

stas commented Aug 27, 2024

Thank you @sosolidkk

  1. Added links Attribute in ErrorSerializer

I don't think links are required, that's why these were left out and I'd prefer to not enforce any members that aren't absolutely required by the spec.

The rest of the changes look good.

@sosolidkk
Copy link
Author

I don't think links are required, that's why these were left out and I'd prefer to not enforce any members that aren't absolutely required by the spec.

The code attribute is also not required but was added. The doc states that an error object MAY have the following members, and MUST contain at least one of those: id, links, status, code, title, detail, source and meta.

If we think about the requirement of those merbers, only the id and status could be enough for most cases.

In any case, if you'd prefer to not enforce it, I can revert the changes and keep only the other two.

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