pypi / warehouse

The Python Package Index
https://pypi.org
Apache License 2.0
3.58k stars 964 forks source link

Add read more or full text link to project license #1354

Open nlhkabu opened 8 years ago

nlhkabu commented 8 years ago

Following on from https://github.com/pypa/warehouse/pull/1284

Where a license is a chunk of text in the DB (and has been concatenated to display on the page), we should add a 'read more' which in turn launches a modal with the full text.

We may also look at making other standard licenses links and launching to the full text in a modal.

Relies on setting up a modal in the CSS/JS, for which I will open a separate issue.

brainwane commented 6 years ago

@Volcyy Thanks for #2917! if you're interested in working on more of Warehouse, could you take a look at this issue? We now have a CSS modal - you can see examples on the "manage project" pages. And you can ping @nlhkabu with any questions or for more detail. You can also ask questions in #pypa-dev on Freenode, or on the pypa-dev mailing list.

jchristgit commented 6 years ago

@brainwane Sure, I'd love to work more on this. I'll look into it!

jchristgit commented 6 years ago

@nlhkabu Alright, I could use some clarification on this: What's supposed to happen is that the license name, e.g. BSD License here: bildschirmfoto 2018-02-14 um 22 45 40 below the Meta section on the left side of the project view is clickable and launches a modal with the full license text? The license text that should launch inside of the modal would be read out from the repository (like the README), or should this be the "standard" text of the license, as seen here (with the BSD License as an example): https://opensource.org/licenses/BSD-2-Clause?

I found that: the project_detailview passes a packaging.models.Release which is linked to [...]Files. My guess is: I'd have to check for a file named LICENSE (or similar) and pass its contents through the template into the modal, or would it be possible for the user to access this? I feel like I'm taking the wrong approach here, so I'd be happy for any pointers.

jchristgit commented 6 years ago

Sorry, I wasn't home for the past weeks; but I finally managed to take a second look at this. I did some testing around and figured out that the File model on Release wasn't quite what I was looking for, since it's compressed data. Is there another way to get the full license text that I've missed? The only thing I've found was the GET /pypi/<project_name>/json but that doesn't seem to be what I'm looking for either. Any pointers for this would be appreciated, given that I'm not going into the wrong direction with this.

di commented 6 years ago

I think this issue might be a bit of a rabbithole, it seems pretty outdated and as @Volcyy has discovered, we don't actually store the full text of the license anywhere in warehouse, just the License Classifiers or the License field.

There's also the issue of the ambiguity of some of the license classifiers that we currently have (e.g. https://github.com/pypa/warehouse/issues/2996) which we will need to resolve first before we can definitively link to the full text of a license.

@nlhkabu Since you are the original creator of this issue, do you mind taking a second look at this and chiming in with your thoughts?

nlhkabu commented 6 years ago

Yes, indeed - I seem to have sent @Volcyy down a rabbit hole here. I believe we used to concatenate the license text, but it looks like we no longer do. I think we can safely close this issue.

@Volcyy - sorry for the misdirection here :( I've just opened a few new tickets that are not too large and definitely needed, so please check out the issue tracker if you'd like to continue contributing to the project :)

di commented 6 years ago

I figured out what this was about: if the License metadata field is set, we are just showing the first line of it, because it could be a multi-line license file (although that's not recommended)

https://github.com/pypa/warehouse/blob/eeabc82178dfcef739df4da09aa37a2aaeefa3e0/warehouse/packaging/views.py#L127-L130

I think it would make sense to somehow "show the rest" of this license, if it's more than one line.

jchristgit commented 6 years ago

Okay, so if we have the full license text given, the user should be able to click the license next to Meta to read the full text by launching a modal? I will take a look at this, but I'm not home again until next weekend, and trying to run this on my laptop is a near-impossible nightmare, sorry 😞. I hope this isn't too urgent.

di commented 6 years ago

Yes, I think if that .split above results in multiple values, then we'll want to add a "read more..." link to the license field we're displaying.

It's not urgent at all! Thanks for working on it.

jchristgit commented 6 years ago

So I'm currently working on this, but I'm a bit unsure as to how to use the modal properly. I took https://github.com/pypa/warehouse/blob/master/warehouse/admin/templates/admin/users/detail.html#L74 as a reference, but when clicking on the link on the license title, nothing happens. Here's the diff from what I have so far:

diff --git a/warehouse/templates/packaging/detail.html b/warehouse/templates/packaging/detail.html
index e389d4d1..08417641 100644
--- a/warehouse/templates/packaging/detail.html
+++ b/warehouse/templates/packaging/detail.html
@@ -152,7 +152,26 @@
         <div class="sidebar-section">
           <h3 class="sidebar-section__title">Meta</h3>
           {% if license %}
-          <p><strong>License:</strong> {{ license }}</p>
+            {% if license_text %}
+              <p><strong>License:</strong> <a data-toggle="modal" data-target="#licenseModal">{{ license }}</a></p>
+              <div class="modal fade" id="licenseModal" role="dialog" tabindex="-1">
+                <div class="modal-dialog" role="document">
+                  <div class="modal-content">
+                      <div class="modal-header">
+                        <h4 class="modal-title">Full license</h4>
+                        <button type="button" class="close" data-dismiss="modal">
+                          <span>&times;</span>
+                        </button>
+                      </div>
+                    <div class="modal-body">
+                      {{ license_text }}
+                    </div>
+                  </div>
+                </div>
+              </div>
+            {% else %}
+              <p><strong>License:</strong> {{ license }}</p>
+            {% endif %}
           {% endif %}
           {% if release.author_email %}
             <p><strong>Author:</strong> <a href="mailto:{{ release.author_email }}">{{ release.author or release.author_email }}</a></p>

@nlhkabu di_codes on IRC suggested that you might be able to answer. I'd be happy for any pointers to get the modal to open. Thanks!

nlhkabu commented 6 years ago

Hi @Volcyy, sorry for the delay on responding to this.

I assume you are working off this branch: https://github.com/Volcyy/warehouse/tree/license-read-more-modal?

Right now, I can't build this, probably because it's out of date with our master branch. Would you be able to update this branch with the latest master, and I can pull it down and take a look? Thanks :)

jchristgit commented 6 years ago

Hey @nlhkabu, the branch you linked is the correct one, and I've just updated it.

nlhkabu commented 6 years ago

@Volcyy @di - is there a specific project on the development database where I can test this?

jchristgit commented 6 years ago

@nlhkabu Thanks for looking into this. hypertools seems to work for me. Using the following in make shell ...

>>> from warehouse.packaging.model import Release
>>> db.query(Release.name).filter(Release.license.like('%\n%').all()

... should return a list of projects with multiline licenses, which is where I found hypertools.