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 groups to access queries #36

Merged
merged 22 commits into from Sep 11, 2019
Merged
Changes from 6 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
9b3acca
[WIP] group ids saving on new reports
mark-vanlandingham-roostify Aug 21, 2019
2132983
Add groups to default queries, and added tab connector
mark-vanlandingham-roostify Aug 22, 2019
3d1b0b3
group_ids set to empty array for default queries
mark-vanlandingham-roostify Aug 22, 2019
cb18b5b
group reports route (in & and) action
mark-vanlandingham-roostify Aug 24, 2019
14f8617
[WIP] created group reports show route/controller
mark-vanlandingham-roostify Aug 29, 2019
06a58b5
Find correct query in show route
mark-vanlandingham-roostify Aug 29, 2019
35e2df1
Removed empty array for group_ids in query file
mark-vanlandingham-roostify Aug 30, 2019
6ab3c60
Add report show view, where users can run queries
mark-vanlandingham-roostify Aug 31, 2019
e11a079
Removed unneeded commas from queries.rb
mark-vanlandingham-roostify Aug 31, 2019
99a788e
Allow non-admin group members to access reports
mark-vanlandingham-roostify Sep 2, 2019
667d239
query-result component dynamic download url based on location
mark-vanlandingham-roostify Sep 2, 2019
29e405d
Removed accidental changes, and corrected tab size
mark-vanlandingham-roostify Sep 2, 2019
e3fd54b
Group members can add params to queries
mark-vanlandingham-roostify Sep 4, 2019
c639670
Specs for new QueryController actions
mark-vanlandingham-roostify Sep 6, 2019
acf7213
remove "Inlude query plan" from group reports
mark-vanlandingham-roostify Sep 6, 2019
8f96cdb
Run prettier
mark-vanlandingham-roostify Sep 6, 2019
0a6cc6e
return and return -> return render
markvanlan Sep 9, 2019
13a78fa
[WIP] changes from review
mark-vanlandingham-roostify Sep 9, 2019
47d6854
Remove weird [-1] group_ids logic, for a simply check for [] in query…
mark-vanlandingham-roostify Sep 9, 2019
6fb16a6
Added integration tests for group report access
mark-vanlandingham-roostify Sep 9, 2019
cc482e0
Using guardian for securing endpoints, and much improved specs
mark-vanlandingham-roostify Sep 10, 2019
3e1707d
Update assets/javascripts/discourse/components/group-reports-nav-item…
markvanlan Sep 10, 2019
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -0,0 +1,14 @@
import { ajax } from "discourse/lib/ajax";

export default {
shouldRender(args, component) {
let usersGroupIds = component.currentUser.groups.map(g => g.id)
if (!usersGroupIds.includes(args.groupId)) return false // User not a part of group

return true
This conversation was marked as resolved by markvanlan

This comment has been minimized.

Copy link
@SamSaffron

SamSaffron Aug 30, 2019

Member

be sure to install prettier locally and always run your files through it. I use IDE integration it is magical.

// ajax(`/explorer/show_reports_tab?group_id=${args.groupId}`)
// .then(showTab => {
// return showTab
// })
}
}
@@ -51,9 +51,28 @@ export default Ember.Controller.extend({
? this.set("showRecentQueries", false)
: this.set("showRecentQueries", true);
if (id < 0) this.set("editDisabled", true);

return item || NoQuery;
},

@computed("selectedItem", "editing")
selectedGroupNames(selectedItem) {
const groupIds = this.selectedItem.group_ids || []
const groupNames = groupIds.map(id => {
return this.groupOptions
.find(groupOption => groupOption.id == id)
.name
})
return groupNames.join(', ')
},

@computed("groups")
groupOptions(groups) {
return groups.query.map(g => {
return {id: g.id.toString(), name: g.name}
});
},

@computed("selectedItem", "selectedItem.dirty")
othersDirty(selectedItem) {
return !!this.model.find(q => q !== selectedItem && q.dirty);
@@ -81,6 +100,10 @@ export default Ember.Controller.extend({
this.set("loading", true);
This conversation was marked as resolved by markvanlan

This comment has been minimized.

Copy link
@SamSaffron

SamSaffron Sep 9, 2019

Member

I think a lot of this can be amended to stop all this .get and .set, cause we this.selectedItem is already assumed to be there in the return. So this.selectedItem.group_ids && this.selectedItem.group_ids.length < 1 and so on in this method make sense.

This comment has been minimized.

Copy link
@markvanlan

markvanlan Sep 9, 2019

Author Contributor

All of this is unnecessary, and has been removed.

if (this.get("selectedItem.description") === "")
this.set("selectedItem.description", "");
// params not comin in
This conversation was marked as resolved by markvanlan

This comment has been minimized.

Copy link
@SamSaffron

SamSaffron Aug 30, 2019

Member

comment a bit confusing

if (!this.get("selectedItem.group_ids") || this.get("selectedItem.group_ids").length < 1)
this.set("selectedItem.group_ids", [-1]) // If this is unset, you cannot remove the last group

return this.selectedItem
.save()
.then(() => {
@@ -212,6 +235,10 @@ export default Ember.Controller.extend({
});
},

cancel() {

},

run() {
if (this.get("selectedItem.dirty")) {
return;
@@ -0,0 +1,13 @@
import Query from "discourse/plugins/discourse-data-explorer/discourse/models/query";
import { popupAjaxError } from "discourse/lib/ajax-error";
import { ajax } from "discourse/lib/ajax";
import {
default as computed,
observes
} from "ember-addons/ember-computed-decorators";

export default Ember.Controller.extend({
actions: {

},
})
@@ -0,0 +1,12 @@
import Query from "discourse/plugins/discourse-data-explorer/discourse/models/query";
import { ajax } from "discourse/lib/ajax";
import {
default as computed,
observes
} from "ember-addons/ember-computed-decorators";

export default Ember.Controller.extend({
queries: Ember.computed.filter('model', function(query) {
return query.group_ids.includes(this.groupId.toString())
}),
})
@@ -0,0 +1,9 @@
export default {
resource: "group",

map() {
this.route("reports", function() {
this.route("show", { path: "/:query_id" });
})
}
}
@@ -23,7 +23,7 @@ const Query = RestModel.extend({
this.resetParams();
},

@observes("name", "description", "sql")
@observes("name", "description", "sql", "group_ids")
markDirty() {
this.set("dirty", true);
},
@@ -85,6 +85,7 @@ Query.reopenClass({
"sql",
"created_by",
"created_at",
"group_ids",
"last_run_at"
]
});
@@ -6,17 +6,21 @@ export default Discourse.Route.extend({
model() {
const p1 = this.store.findAll("query");
This conversation was marked as resolved by markvanlan

This comment has been minimized.

Copy link
@SamSaffron

SamSaffron Sep 10, 2019

Member

probably worth renaming this to queryPromise and so on ... p1,p2,p3 is somewhat confusing.

const p2 = ajax("/admin/plugins/explorer/schema.json", { cache: true });
const p3 = ajax("/admin/plugins/explorer/groups.json"); // Should cache be true?
This conversation was marked as resolved by markvanlan

This comment has been minimized.

Copy link
@SamSaffron

SamSaffron Aug 30, 2019

Member

This will not cache ... its not really a cachable route, one consideration here is preload store, may make sense to use it. or amend serializer to include groups

This comment has been minimized.

Copy link
@markvanlan

markvanlan Sep 2, 2019

Author Contributor

Using existing group api now

return p1
.then(model => {
model.forEach(query => query.markNotDirty());

return p2.then(schema => {
return { model, schema };
return p3.then(groups => {
return { model, schema, groups };
})
});
})
.catch(() => {
p2.catch(() => {});
return { model: null, schema: null, disallow: true };
p3.catch(() => {});
return { model: null, schema: null, disallow: true, groups: null };
});
},

@@ -0,0 +1,32 @@
import { ajax } from "discourse/lib/ajax";

export default Discourse.Route.extend({
controllerName: "group-reports",

model() {
// console.log(this.controller.target.parent.params.name)
const groupId = this.modelFor('group').id
const p1 = this.store.findAll("query");
return p1
This conversation was marked as resolved by markvanlan

This comment has been minimized.

Copy link
@SamSaffron

SamSaffron Sep 10, 2019

Member

no need to define p1 here, return ajax(...

.then(queries => {
return {
model: queries,
groupId: groupId
}
})
.catch(() => {
return { model: null };
});
},

setupController(controller, model) {
controller.setProperties(model);
},

actions: {
refreshModel() {
this.refresh();
return false;
}
}
});
@@ -0,0 +1,26 @@

import { ajax } from "discourse/lib/ajax";

export default Discourse.Route.extend({
controllerName: "group-reports-show",

model(params) {
// console.log(this.controller.target.parent.params.name)
const p1 = this.store.find("query", parseInt(params.query_id));
return p1
.then(query => {
return { model: query }
})
},

setupController(controller, model) {
controller.setProperties(model);
},

actions: {
refreshModel() {
this.refresh();
return false;
}
}
});
@@ -38,6 +38,7 @@
<div class="desc">
{{textarea value=selectedItem.description placeholder=(i18n "explorer.description_placeholder")}}
</div>

{{else}}
<div class="name">
{{d-button action=(action "goHome") icon="chevron-left" class="previous"}}
@@ -52,6 +53,20 @@
</div>
{{/if}}

<div class="groups">
<span class="label">Allow groups to acess this query</span>
<span>
{{ multi-select values=selectedItem.group_ids content=groupOptions }}
This conversation was marked as resolved by markvanlan

This comment has been minimized.

Copy link
@eviltrout

eviltrout Sep 9, 2019

Member
Suggested change
{{ multi-select values=selectedItem.group_ids content=groupOptions }}
{{multi-select values=selectedItem.group_ids content=groupOptions}}
</span>
{{#if runDisabled }}
This conversation was marked as resolved by markvanlan

This comment has been minimized.

Copy link
@eviltrout

eviltrout Sep 9, 2019

Member
Suggested change
{{#if runDisabled }}
{{#if runDisabled}}
{{#unless editing}}
<span class='setting-controls'>
{{d-button class="ok" action=(action "save") icon="check"}}
{{d-button class="cancel" action=(action "discard") icon="times"}}
</span>
{{/unless}}
{{/if}}
</div>
{{! the SQL editor will show the first time you }}
{{#if everEditing}}
<div class="query-editor {{if hideSchema "no-schema"}}">
@@ -0,0 +1,7 @@
<ul class ='nav-pills'>
<li>
{{#link-to 'group.reports'}}
{{i18n 'group.reports'}}
{{/link-to}}
</li>
</ul>
@@ -0,0 +1,25 @@
<section class='user-content'>
<table class='group-members'>
<thead>
<th>
{{i18n "explorer.report_name"}}
</th>
<th>
{{i18n "explorer.query_description"}}
</th>
<th>
{{i18n "explorer.query_time"}}
</th>
</thead>
<tr></tr>
<tbody>
{{#each queries as |query|}}
<tr>
<td><a href="reports/{{query.id}}">{{query.name}}</a></td>
This conversation was marked as resolved by markvanlan

This comment has been minimized.

Copy link
@eviltrout

eviltrout Sep 9, 2019

Member

Can you use {{link-to}} here? If not this link will need to be aware of subfolder installations of Discourse. (usually a call to Discourse.getURL

This comment has been minimized.

Copy link
@markvanlan

markvanlan Sep 9, 2019

Author Contributor

We can!

<td>{{query.description}}</td>
<td>{{bound-date query.last_run_at}}</td>
</tr>
{{/each}}
</tbody>
</table>
</section>
@@ -0,0 +1 @@
<h1>{{model.name}}</h1>
@@ -185,6 +185,16 @@
&:not(.editing) .desc {
margin: 10px 0;
}
.groups {
margin-bottom: 10px;
.label {
margin-right: 10px;
color: $primary-high;
}
.name {
display: inline;
}
}
}

.query-run {
@@ -59,6 +59,7 @@ en:
one: "Showing top %{count} results."
other: "Showing top %{count} results."
query_name: "Query"
report_name: "Report"
query_description: "Description"
query_time: "Last run"
query_user: "Created by"
@@ -68,3 +69,6 @@ en:
reset_params: "Reset"
search_placeholder: "Search..."
no_search_results: "Sorry, we couldn't find any results matching your text."
group:
reports: "Reports"

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.