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

Implement hidden state for versions #6792

Merged
merged 33 commits into from Apr 28, 2020
Merged

Conversation

stsewd
Copy link
Member

@stsewd stsewd commented Mar 18, 2020

Ref #6194 and #5321

This doesn't remove the protected privacy level yet (we can do that after this change is released).

Changes:

  • Add hidden field
  • Change search and footer to respect the hidden state
  • Add the hidden field to API v3
  • Migrate all protected versions to be hidden

@stsewd stsewd requested a review from Mar 18, 2020
@stsewd
Copy link
Member Author

@stsewd stsewd commented Mar 18, 2020

Should we filter these from the sitemap as well?

Copy link
Member

@ericholscher ericholscher left a comment

This looks like a good start. I feel like we need a bit more explanation for users before we really ship this, since it can be a bit confusing I think.

docs/versions.rst Outdated Show resolved Hide resolved
docs/versions.rst Outdated Show resolved Hide resolved
docs/versions.rst Outdated Show resolved Hide resolved
readthedocs/search/api.py Outdated Show resolved Hide resolved
stsewd and others added 3 commits Mar 24, 2020
Co-Authored-By: Eric Holscher <25510+ericholscher@users.noreply.github.com>
@stsewd
Copy link
Member Author

@stsewd stsewd commented Mar 24, 2020

I feel like we need a bit more explanation for users before we really ship this, since it can be a bit confusing I think.

Do you mean like improving the UX or communicating the users with a blog post/email?

@ericholscher
Copy link
Member

@ericholscher ericholscher commented Mar 24, 2020

Do you mean like improving the UX or communicating the users with a blog post/email?

Improving the on-site copy and docs. Perhaps some general uses cases and examples, and a Guide (eg. how do I hide a version and keep docs online?).

I think the main thing is "I'm a user and I see this "hidden" option, how do I understand what it actually does".

@stsewd
Copy link
Member Author

@stsewd stsewd commented Mar 24, 2020

@ericholscher I expanded the docs and wrote a small guide https://docs--6792.org.readthedocs.build/en/6792/guides/hiding-a-version.html

And used crispy forms to group the elements on the UI. Let me know what you think.

Screenshot_2020-03-24 stable - test Read the Docs

@stsewd
Copy link
Member Author

@stsewd stsewd commented Mar 24, 2020

Oh, and one thing. This migration is adding a non-nullable field, so when deploying users aren't going to be able to create new versions during the deploy.

We can avoid the downtime by setting the new field as nullable and:

  • run the migration that adds the new field
  • deploy the code
  • run the data migration to migrate all protected versions to hidden=True and update all versions with hidden=None to hidden=False
  • Make the field non-nullable in the next deploy

I also saw this package that solves the problem without doing all those steps https://github.com/3YOURMIND/django-add-default-value but I haven't tested it

@stsewd stsewd requested a review from ericholscher Mar 25, 2020
Copy link
Member

@ericholscher ericholscher left a comment

We can avoid the downtime by setting the new field as nullable and:

I think this is a good idea 👍

docs/guides/hiding-a-version.rst Outdated Show resolved Hide resolved
Go to the :guilabel:`Versions` tab of your project, click on :guilabel:`Edit` and mark the ``Hidden`` option.

Users that have a link to your old version will still be able to see your docs.
And new users can see all your versions (including hidden versions) in the versions tab of your project at ``https://readthedocs.org/projects/<your-project>/versions/``
Copy link
Member

@ericholscher ericholscher Apr 16, 2020

Choose a reason for hiding this comment

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

Do we actually want this? It doesn't make hidden very hidden, I think.

Copy link
Member

@humitos humitos Apr 17, 2020

Choose a reason for hiding this comment

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

Yeah. However, if we hide the versions from there, you can't un-hide them. Although, we could show hidden versions in that list if you are an admin of the project.

Copy link
Member Author

@stsewd stsewd Apr 20, 2020

Choose a reason for hiding this comment

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

we could show hidden versions in that list if you are an admin of the project.

Hmm, yeah. Not sure, I mean, users will only be able to get there by search results on google or old links, not from the project. I'll create an issue to discuss that.

Copy link
Member Author

@stsewd stsewd Apr 20, 2020

Choose a reason for hiding this comment

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

@ericholscher
Copy link
Member

@ericholscher ericholscher commented Apr 16, 2020

I'd like to not break versions during deploy, so merge after we edit the migration

docs/versions.rst Show resolved Hide resolved
docs/versions.rst Show resolved Hide resolved
Copy link
Member

@humitos humitos left a comment

Looks good.

I left some important questions about privacy levels on search and the migration file.

docs/versions.rst Show resolved Hide resolved
readthedocs/builds/models.py Show resolved Hide resolved
readthedocs/search/api.py Show resolved Hide resolved
stsewd and others added 3 commits Apr 20, 2020
@stsewd
Copy link
Member Author

@stsewd stsewd commented Apr 20, 2020

Would the appear in the sitemap.xml, robots.txt? I may worth to mention these things here as well.

I think there was a discussion on slack around listing this in robots.txt, currently we only put this as default

return HttpResponse(
'User-agent: *\nAllow: /\nSitemap: {}\n'.format(sitemap_url),
content_type='text/plain',
)

About the sitemap, that still needs an answer #6792 (comment). Currently, we are listing those in the sitemap. /cc @ericholscher

@humitos
Copy link
Member

@humitos humitos commented Apr 21, 2020

I think they should not appear in the sitemap.xml and should be Disallow in robots.txt. IMO that's aligned with the meaning of hidden: you don't want people to find these versions.

@ericholscher
Copy link
Member

@ericholscher ericholscher commented Apr 27, 2020

Yea, definitely they be Disallowed in the robots.txt. I don't feel strongly about sitemap. Is there a use for the sitemap.xml that isn't search indexing? Hidden isn't private, so I think showing them in the sitemap and just asking the crawlers not to index them seems fine.

readthedocs/templates/robots.txt Outdated Show resolved Hide resolved
@stsewd stsewd requested a review from ericholscher Apr 28, 2020
@stsewd
Copy link
Member Author

@stsewd stsewd commented Apr 28, 2020

Active versions that are hidden are listed in the robots.txt file now.

Copy link
Member

@ericholscher ericholscher left a comment

This looks 💯 to me. Excited to ship it!

@ericholscher ericholscher merged commit fc8adb7 into master Apr 28, 2020
2 checks passed
@ericholscher ericholscher deleted the implement-hidden-state-for-versions branch Apr 28, 2020
Copy link
Member

@humitos humitos left a comment

👍

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.

None yet

3 participants