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

fix in custom schema (rest api) #1490

Merged
merged 4 commits into from
May 9, 2018

Conversation

waychal
Copy link
Contributor

@waychal waychal commented May 7, 2018

Fixes #1390

@waychal waychal requested a review from ltalirz May 7, 2018 20:42
@ltalirz
Copy link
Member

ltalirz commented May 7, 2018

@waychal First of all, thanks for the fix!

Is the idea that the definition of the projections will move from the custom schema inside the _default_projections of the translator class?
I guess this seems like a reasonable place to define them to make sure that people don't forget them when they add a new translator.

@codecov-io
Copy link

codecov-io commented May 7, 2018

Codecov Report

Merging #1490 into release_v0.12.1 will increase coverage by <.01%.
The diff coverage is 65.62%.

Impacted file tree graph

@@                 Coverage Diff                 @@
##           release_v0.12.1    #1490      +/-   ##
===================================================
+ Coverage            54.42%   54.42%   +<.01%     
===================================================
  Files                  246      246              
  Lines                32454    32478      +24     
===================================================
+ Hits                 17663    17677      +14     
- Misses               14791    14801      +10
Impacted Files Coverage Δ
aiida/restapi/translator/node.py 46.89% <37.5%> (-0.33%) ⬇️
aiida/restapi/translator/user.py 80.95% <75%> (-5.72%) ⬇️
aiida/restapi/translator/group.py 80.95% <75%> (-5.72%) ⬇️
aiida/restapi/translator/computer.py 85.71% <75%> (-7.62%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 659bd6b...48a2d97. Read the comment docs.

@waychal
Copy link
Contributor Author

waychal commented May 8, 2018

@ltalirz part of the custom_schema will move to the ORM classes like help_text and display_name as they are more related to the node. is_display and ordering is required only in rest api.. so that part might go to the translator. I still need to finalise that.

@waychal
Copy link
Contributor Author

waychal commented May 8, 2018

@ltalirz there is separate issue for moving custom_schema to orm classes and it requires time to solve that. Until then I guess its good to marge this pr to make the functionality working in current code.

Copy link
Member

@ltalirz ltalirz left a comment

Choose a reason for hiding this comment

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

Sounds good!

@waychal waychal merged commit d3ebdf1 into aiidateam:release_v0.12.1 May 9, 2018
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