awesomemotive / easy-digital-downloads

Sell digital downloads through WordPress
https://easydigitaldownloads.com
GNU General Public License v2.0
867 stars 471 forks source link

Add download link in admin area when folder is not accessed with http #3002

Open abeel opened 9 years ago

abeel commented 9 years ago

for each product let the admin to download the file if the edd download folder isnot accessed with http (outside the site folder)

File URL : /var/downloads/download/edd/2015/01/download.zip

pippinsplugins commented 9 years ago

We've had this requested a few times and I think it's a good idea.

cklosowski commented 9 years ago

I must be missing something. Can you e plain the use case @pippinsplugins?

pippinsplugins commented 9 years ago

If you copy and paste the file URLs from the edit screen of a Download product, you will be given a "Permission denied" error message due to the htaccess file.

Imagine a site using Frontend Submissions. One of the ways that site admins review files is by downloading them. With the current system, admins are required to download them via FTP to view the files.

abeel commented 9 years ago

@pippinsplugins +1

cklosowski commented 9 years ago

Ah. Perfect, thanks. :100:

evertiro commented 9 years ago

What do we want this to look like?

pippinsplugins commented 9 years ago

Take a look at WPeC, they have a pretty nice system: screen shot 2015-01-27 at 3 09 45 pm

evertiro commented 9 years ago

Do we only want it to show if the downloads aren't accessible view http, or always show it?

pippinsplugins commented 9 years ago

Show regardless.

cklosowski commented 9 years ago

image @pippinsplugins You think this should do it? Could use some testing for Dropbox, Amazon, etc.

cklosowski commented 9 years ago

Also adding logging and Nonce verification. Now when an Admin downloads the file: image

pippinsplugins commented 9 years ago

@cklosowski A few things:

  1. I don't think this should be a separate meta box. It should somehow be incorporated into the existing Download Files metabox, otherwise it creates a disconnect and will probably cause confusion. "Why are there files here and there?"
  2. If adding a download processor for admins, we should extract as much out of the existing ones so that both admin/customer methods share code instead of duplicating it.
cklosowski commented 9 years ago

@pippinsplugins 1) Totally disagree, I think it offers more confusion if it's just below in the same metabox. Plus it starts making that a 'God Metabox', just WAY too much going on in it. 2) I was pretty conservative in what I ripped out, I'll try to rip a few more things into a function.

pippinsplugins commented 9 years ago

1). Ignoring possible issues with making the metabox too large, can you give me your reasons for feeling that a separate box is less confusing?

cklosowski commented 9 years ago

1) It has a separate permission than the regular metabox 2) It makes it possible to hide it, if you aren't running FES or the use case isn't valid for you 3) We don't show it if it's part of bundles, this means if it's part of the other metabox, you have too add more html that may (or may not) ever display and then ajax must update it. If it's a bundle, we just don't need to show it.

cklosowski commented 9 years ago

VOTE! Camera 1: Cleans up the delete column to include a download icon and is part of the exiting files metabox image

Camera 2: Adds a new metabox to show the attached files and some info about them image

pippinsplugins commented 9 years ago

@easydigitaldownloads/core-devs What do you think guys? Which option do you prefer?

SeanTOSCD commented 9 years ago

Uggghhh I really like the separate metabox and the reasons for having it. But I think having the actions column in the existing metabox makes more sense for those who will actually use it.

cklosowski commented 9 years ago

I read that as Sean likes both ;-)

mindctrl commented 9 years ago

Option 1. Less change. Nice icons, though they might be too close and someone could accidentally delete instead of download.

With space being an issue in this box, maybe we should consider shortening the URL box or coming up with an interesting way to hide the URL by default and show it with a click or something. It rarely, if ever, shows the whole URL anyway.

pippinsplugins commented 9 years ago

Is no one else concerned with the possibility of people getting confused y two meta boxes? Even as a power user, having data about the file downloads in two places seems super confusing to me.

SeanTOSCD commented 9 years ago

@pippinsplugins I'm concerned with it. That's why I chose Camera 1.

To avoid getting too cluttered in the metabox, assuming we go with Camera 1, consider combining the File Name and File URL columns. Imagine this:

One column called File Details that is the width of the current File URL column and is positioned all the way left. Stretch each row vertically just a liiiittle bit and add (in smaller, bold text) the name of the file above the File URL field. Click-to-edit the file name (maybe a pencil icon nearby).

That frees up 20-25% horizontal space in the metabox and allows us to use Camera 1, which again I think makes more sense.

pippinsplugins commented 9 years ago

File name and URL cannot be combined. People manually customize the file name some times.

cklosowski commented 9 years ago

We can add the table of file details to the other meta box however I'm getting concerned with the maintainability of that metl box from a code perspective

SeanTOSCD commented 9 years ago

I already said click-to-edit the file name in my last reply.

2015-06-15 at 3 38 pm

cklosowski commented 9 years ago

I think the download information is important to show. Let me try something, against my opinion, but adding some of this information to the same metabox.

cklosowski commented 9 years ago

Punting for further discussion as there is clearly some disagreements in what this should be, and how it should function.

cklosowski commented 9 years ago

Just something to bring up, if we can get away from using Tables for the file list, this will open up a lot of possibilities in what data we can display via show/hide links. Right now it's restricted to a <tr> which isn't really easy to expand vertically.

We could convert the repeatable row to be divs, and that would help us go a long way with expanding it's extensibility. The issue however is a few hooks into the repeatable row that are expecting the hook to be within a tr

rubengc commented 7 years ago

A possible solutions could be add row actions under file URL column (similar to wordpress row actions) with upload/download/clear actions

cklosowski commented 7 years ago

So with the work that @SDavisMedia did on #2704, I think we can compleatly rework the repeatable file fields to work the same way, with an 'Advanced Options' (or some other wording like "File Information") link that drops down an area within the existing metabox for the Files for a download.

cklosowski commented 7 years ago

Ok, I think I have this reworked using the repeatable fields 'info' sections, as well as logging file downloads as admins (wether it's a local or remote file).

Here's a screenshot of the implementation, and like the 'Advanced Settings' line for variable pricing, there is a link to 'view file information'. screen shot 2017-08-28 at 12 22 05 pm

This is ready for some testing.

chriscct7 commented 7 years ago

I personally think "WordPress" for on-site and "External Site" for off-site would be less confusing. "Local" could mean the current user's computer and "remote" I'm not sure means much to a non-technical user. Then Amazon s3 for instance could hook in and change the label for their files for "Amazon S3". To me that it makes it more clear to the average user where the file is.

Otherwise :+1:

cklosowski commented 7 years ago

Thanks. Will look at options. I think WordPress is a bit too general because it could be an FTPd file. So maybe just the site URL?

On Aug 28, 2017 4:18 PM, "Chris Christoff" notifications@github.com wrote:

I would change File Location where the value is "Local" to "Uploaded" or "WordPress" or something. "Local" could be interpreted as it's on the computer of the person whose currently logged in.

I personally think "WordPress" for on-site and "External Site" for off-site would be less confusing. Then Amazon s3 for instance could hook in and change the label for their files for "Amazon S3"

Otherwise 👍

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/easydigitaldownloads/easy-digital-downloads/issues/3002#issuecomment-325509603, or mute the thread https://github.com/notifications/unsubscribe-auth/ABU2LB4HotBlew1-E1LyYSJmDf7o2OLRks5sc0regaJpZM4DXL8q .

chriscct7 commented 7 years ago

Well a normal user isn't going to think FTP vs WP Media will matter as to where the site is. Is the point to say it's not in wp-content if it's not? Does it really matter in this area to the user if the file is FTP'd in vs media library? As long as the download link works, I don't think the avg WP user really will care.

If so maybe:

cklosowski commented 7 years ago

@chriscct7 Since the file input field supports relative URL, full file path, and media library, I would think just stating it's delivered via your site site_url would be enough here. Then we can add a filter for things like S3 and Dropbox to change that in a future update.

chriscct7 commented 7 years ago

That's fair, though the site urls can get really long. I still think just "Website" would be fine. The file location regardless of if its ftp'd or whatever is still served from this website.

mindctrl commented 7 years ago

Why do you need to show File Location? The File URL basically reveals that already.

chriscct7 commented 7 years ago

I agree with John for the most part, though it could be nice for Amazon s3 users to see that a file is uploaded to s3 or not, particularly if someone just installed S3 and is trying to quickly find files they need to re-upload to s3.

mindctrl commented 7 years ago

Since the heading above is File Information, repeating "File" on every item below it (File Location, File Size, etc) feels redundant.

mindctrl commented 7 years ago

Doesn't S3 show s3:// or something like that on URLs?

chriscct7 commented 7 years ago

Yeah but how many users will know to look for that in a URL? Plus it's not as easy to see, particularly on something like a mobile device, or when quickly scanning through many edit download pages.

The file repeat thing I agree with in premise though you could argue the whole box is supposed to be information about the file, so you could easily remove the word "file" from every box except the title of the metabox itself. The prefix of "file" repetition in this case I don't think really detracts from the user experience. It makes it easier to quickly scan and find items, versus looking for the "file size" and seeing "size" and then having to back-inference it to meaning "file size"

cklosowski commented 7 years ago

@mindctrl no, not all S3 files show the s3:// handler. Some simply are just / for the bucket location.

cklosowski commented 7 years ago

And as stated above, the file location may be served from the local site, but might not be a fully qualified URL. It could simply be /var/www/path/to/file.txt or /wp-content/uploads/file.zip, which don't reference the actual place it's being served from by the normal user.

cklosowski commented 7 years ago

@chriscct7 I understand, but that's why file location can be useful.

mindctrl commented 7 years ago

It seems like it's solving a problem caused by the URL field being way too short to show anything useful about the file already uploaded and selected.

cklosowski commented 7 years ago

@mindctrl not true, again, files that are not fully qualified URLs, no matter how large the textbox is, can be confusing to a normal user on where they are being delivered from, since there is no reference to the location of the file in reference to the site.

cklosowski commented 7 years ago

For instance, here is an S3 file (need to work out getting the file information for it still on S3), that the URL section is pretty confusing for an end user who doesn't know how S3 works on a technical URL level.

screenshot upload fail...here's the good one screen shot 2017-08-28 at 4 49 09 pm

cklosowski commented 7 years ago

As a note, i'm still working out the kinks with the file storage solutions as far as delivery and file information.

chriscct7 commented 7 years ago

But what user, outside of like Amazon s3, actually use a non-qualified URL and then do not realize it will be served from the same domain? I feel like Amazon s3 should just indicate the file is on AWS and then remove the file location thing and call it a day. Frankly the file type and file size probably aren't useful, because if the file url was longer, then you'd know what file type it is, and the file size is extraneous to most users.

2 alternative ideas: No minimized area image Minimized Area image

chriscct7 commented 7 years ago

Or you could do something like this: image or image