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

Jkmarx/groups api v2 #3265

Merged
merged 9 commits into from Mar 21, 2019
Merged

Jkmarx/groups api v2 #3265

merged 9 commits into from Mar 21, 2019

Conversation

jkmarx
Copy link
Member

@jkmarx jkmarx commented Mar 19, 2019

Ref #2618

  • Add a group API V2 which supports query by dataSetUuid (will extend in the next few pull requests)
  • Extend data set API V2 to return owner object and user_perms (previously sent by share api)
  • ? Consider removing is_owner field in the future
  • Update client to use the groups api to get groups with data set's perms
    • Had to reorganize client and rename components to better fit new api calls
  • Add unit tests

Can't remove data set resource until we extend group api v2 to include patch for updating perms.

@codecov
Copy link

codecov bot commented Mar 19, 2019

Codecov Report

Merging #3265 into develop will decrease coverage by 1.11%.
The diff coverage is 77.57%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #3265      +/-   ##
===========================================
- Coverage    66.66%   65.55%   -1.12%     
===========================================
  Files          386      383       -3     
  Lines        26659    25277    -1382     
  Branches       976      947      -29     
===========================================
- Hits         17773    16570    -1203     
+ Misses        8886     8707     -179
Impacted Files Coverage Δ
refinery/ui/source/js/file-browser/ctrls/ctrl.js 34.9% <10%> (-1.33%) ⬇️
...data-set-visualization/ctrls/visualization-ctrl.js 66.66% <100%> (ø) ⬆️
refinery/ui/source/js/commons/services/group.js 100% <100%> (+11.11%) ⬆️
refinery/core/test_views.py 100% <100%> (ø) ⬆️
refinery/core/serializers.py 84.67% <100%> (-7.57%) ⬇️
refinery/core/urls.py 100% <100%> (ø) ⬆️
...e-browser/services/data-set-group-perms-service.js 100% <100%> (ø) ⬆️
...source/js/data-set-about/directives/group-perms.js 100% <100%> (ø) ⬆️
.../ui/source/js/data-set-about/ctrls/details-ctrl.js 45.07% <46.37%> (-11.42%) ⬇️
.../source/js/file-browser/services/data-set-props.js 72% <50%> (-8.56%) ⬇️
... and 9 more

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 1a671b1...af1d299. Read the comment docs.

@jkmarx jkmarx self-assigned this Mar 20, 2019
@jkmarx jkmarx added this to the Release 1.6.9 milestone Mar 20, 2019
@@ -1,142 +1,145 @@
'use strict';
/**
Copy link
Member Author

Choose a reason for hiding this comment

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

reorganized to meet style guide and use new api

@@ -0,0 +1,45 @@
/**
Copy link
Member Author

Choose a reason for hiding this comment

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

Switched over the share_component to group_perms_component name, due to new apis.

@@ -468,7 +468,33 @@ <h3>Data Files</h3>
</span>
</p>

<rp-data-set-about-sharing></rp-data-set-about-sharing>
<div class="refinery-header">
Copy link
Member Author

Choose a reason for hiding this comment

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

Now returned from data_set v2 api so moved into details component

@jkmarx jkmarx requested a review from hackdna March 20, 2019 12:52
self.data_set.share(self.group)
self.data_set.share(self.group_2)

def test_get_groups_with_ds_uuid_returns_401_for_anon(self):
Copy link
Member

Choose a reason for hiding this comment

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

I'd expand ds to dataset to make it more explicit (here and below).

def setUp(self):
super(GroupApiV2Tests, self).setUp(
api_base_name="groups/",
view=GroupViewSet.as_view({'get': 'list'})
Copy link
Member

Choose a reason for hiding this comment

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

These two could fit on one line?

@jkmarx jkmarx merged commit 42213ca into develop Mar 21, 2019
@jkmarx jkmarx deleted the jkmarx/groups-api-v2 branch March 21, 2019 17:08
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