Skip to content
This repository has been archived by the owner on Apr 22, 2024. It is now read-only.

Provide access to dataset logbook #699

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Provide access to dataset logbook #699

wants to merge 5 commits into from

Conversation

nitrosx
Copy link
Contributor

@nitrosx nitrosx commented Oct 31, 2022

Description

This PR enables users with access to dataset or admin privileges to view the logbook associated with the dataset itself.
A new function has been added to the logbook model and a new endpoint to the dataset which returns the logbook associated with it if it exists.

Motivation

In the previous version, only dataset owners could see the logbook. Not even admin could see it.

Tests included/Docs Updated?

  • Included for each change/fix?
  • Passing? (Merge will not be approved unless this is checked)
  • Docs updated?
  • New packages used/requires npm install?
  • Toggle added for new features?

common/models/dataset.js Show resolved Hide resolved
where: { ownerGroup: { inq: options.currentGroups } },
});
let proposals = {};
if ( options.currentGroups.includes("admin") ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do not think so, at least, I did not find this feature.
Before my change, access to the logbook was still managed locally. I juts extended the logic to include access groups.

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought that the find of every method which inherits from ownable calls the before("access") operation hook, thus the lines that I've shared. The check there would be on global access though rather than admin

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants