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

Adding audience to panes and filtering based on that and user roles #168

Merged
merged 6 commits into from Sep 2, 2019

Conversation

megoth
Copy link
Contributor

@megoth megoth commented Aug 29, 2019

This is dependent on solid/solid-ui#112, so do not merge this until it has been merged and released and the PR has been updated with corresponding release version.

Making it easier to add extra logic that takes what kind of user the user is
This is dependent on solid/solid-ui#112, so do not merge this until it has been merged and released and the PR has been updated with corresponding release version.
@megoth megoth requested review from Vinnl and timbl August 29, 2019 13:29
@megoth megoth added this to To do in Data Browser via automation Aug 29, 2019
@megoth
Copy link
Contributor Author

megoth commented Aug 29, 2019

This PR should also include an update to issue-pane after SolidOS/issue-pane#1 has been merged.

@megoth
Copy link
Contributor Author

megoth commented Aug 29, 2019

This PR should also include an update to meeting-pane after SolidOS/meeting-pane#1 has been merged.

@megoth
Copy link
Contributor Author

megoth commented Aug 29, 2019

This PR should also include an update to folder-pane after SolidOS/folder-pane#3 has been merged.

@megoth megoth moved this from To do to Needs review in Data Browser Aug 29, 2019
@megoth
Copy link
Contributor Author

megoth commented Aug 29, 2019

This is part of fixing #167

Copy link
Contributor

@timbl timbl left a comment

Choose a reason for hiding this comment

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

It is important that when a user is looking at an object (like a meting or a bank transaction ) where there is a hand-coded view, then they must be given that view, whether or not they are an expert user. It is very bad to deprive them of the proper view and give them a raw data view! We can I suppose detect that as the highest priority pane in the way the pane system works at the moment, though we cloudl cahnge that to haveing q values for each pane.

Copy link
Contributor

@Vinnl Vinnl left a comment

Choose a reason for hiding this comment

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

LGTM; setting it to "Request changes" to make sure the upgrade to a new major version of solid-ui with https://github.com/solid/solid-ui/pull/112 is included.

defaultPane.js Show resolved Hide resolved
outline/manager.js Outdated Show resolved Hide resolved
@megoth megoth moved this from Needs review to In progress in Data Browser Aug 29, 2019
Should've been removed in earlier commit
Handle the case that Tim describes in #168 (review)
@megoth megoth moved this from In progress to On hold in Data Browser Aug 29, 2019
@megoth
Copy link
Contributor Author

megoth commented Aug 29, 2019

@Vinnl and @timbl can you accept the reviews, so that I can do the appropriate merges?

@megoth megoth moved this from On hold to Needs review in Data Browser Aug 29, 2019
@megoth megoth added the release-minor Add to PR to indicate that merging it denotes a minor (semver) release label Aug 29, 2019
@@ -380,8 +380,21 @@ module.exports = function (doc) {

async function getRelevantPanes (panes, subject, dom) {
const relevantPanes = panes.list.filter(pane => pane.label(subject, dom) && !pane.global)
if (relevantPanes.length === 0) {
// there are no relevant panes, simply return default pane (which ironically is internalPane)
Copy link
Contributor

Choose a reason for hiding this comment

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

¿¿¿ eh??? The default pane and the internals pane should be quite different. The default pane should the normal properties in a list, excluding things deemed to be internal like the URI itself, the record of attempts to load the URI etc. Those things are shown in the internals pane, which also allows you to refresh the local triple store, delete the file, etc. The default pane is not the internal pane.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The irony is in the naming - defaultPane sounds like it should be the default pane that is shown.

@megoth megoth moved this from Needs review to Done in Data Browser Sep 2, 2019
@megoth megoth merged commit 2892c38 into master Sep 2, 2019
@megoth megoth deleted the user-roles branch September 2, 2019 11:30
@megoth megoth moved this from Done to Release 2 in Data Browser Sep 2, 2019
@brownhoward brownhoward added this to the Data Browser Release 2 milestone Oct 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-minor Add to PR to indicate that merging it denotes a minor (semver) release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants