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

Perposal for some changes to dpd-fileupload #59

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

Conversation

tenowg
Copy link

@tenowg tenowg commented May 7, 2016

I have made a few changes to how this Module/Collection works on a base level, creating a config page, implementing a ability to require a login to write a file, implementing Upload event, implemented an extension for dashboard so data shows correctly.

If you want editable properties, you can add a properties.html/properties.js copied from collection/dashboard and modify them for this module.

As a caveat to Module creation... I created the modules while using npm link, __dirname was incorrect, so I used a directory based off config location.

tenowg and others added 2 commits May 7, 2016 01:40
@tenowg
Copy link
Author

tenowg commented May 7, 2016

I have tested this locally and all works well, but travis is failing, so I will look over the tests when I have time...

@NicolasRitouet
Copy link
Owner

Hi @tenowg
Thanks for your proposal.

Give me some time to review this.

In the meanwhile, can you make the tests pass?

@tenowg
Copy link
Author

tenowg commented May 10, 2016

I will have a few hours to work on this in the next few days... But I have been playing around with this more than I should be... I will push what I have to my organization, and I will attempt to make all tests pass... I believe the fault is the permissions section

if (!this.config.properties || !this.config.directory || !this.config.fullDirectory) {
// We will no longer need a config.properties
// This 'should' be first run only, and shouldn't need to be saved...
if (!this.config.directory || !this.config.fullDirectory) {
Copy link
Author

Choose a reason for hiding this comment

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

will be removing this part all together shortly... config will be set with this.config.blah || process.env.blah || "default"... saving the need for a automatic forced save

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

Successfully merging this pull request may close these issues.

2 participants