Re-sync Repository from VCS #6210

saadmk11 · 2019-09-26T13:49:06Z

No description provided.

stsewd

Looks good to me. I think the button should be placed in another place, like above Active versions, to the right.

readthedocs/templates/projects/project_version_list.html

saadmk11 · 2019-09-26T18:16:00Z

@stsewd This looks better I think :)

humitos

Thanks! The PR looks good to me.

I left some suggestions about class naming and button label. Also, I think that we should move the button below the "Active Versions" title, to keep consistency with the "Builds" page, for example:

readthedocs/projects/urls/private.py

readthedocs/projects/views/private.py

readthedocs/templates/projects/project_version_list.html

Co-Authored-By: Manuel Kaufmann <[email protected]>

humitos

LGTM! Thanks for taking the suggestions!

stsewd · 2019-10-10T15:30:40Z

I think that we should move the button below the "Active Versions" title, to keep consistency with the "Builds" page, for example:

The difference is that the sync versions button doesn't apply only for active versions, but for inactive versions as well.

humitos · 2019-10-11T09:01:13Z

The difference is that the sync versions button doesn't apply only for active versions, but for inactive versions as well.

You may have a point. Although, having a button outside the title of the page didn't look good to me: seems broken style.

The "Sync" does not affect "Active versions", it only affect "Inactive versions" for now --because there is no automated rules yet. So, at this moment, the right place would be under "Inactive versions".

We do need to place it somewhere, and I'm not seeing a perfect place for it. I suggest to leave it below "Active versions".

stsewd · 2019-10-11T14:25:29Z

The "Sync" does not affect "Active versions"

Actually it does, it activates a version if it follows semver and it's greater than the current stable. Also it updates latest and stable.

humitos · 2019-10-11T18:14:55Z

OK. So, it makes more sense to be below the "Active versions" then 😄

stale · 2019-11-25T18:55:19Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

ericholscher · 2019-11-26T09:57:10Z

I think this is a good interim UI, but I don't think it's quite ready. I think the better approach here is to make it an actual part of the page, with copy that explains what it does. Currently "Sync Versions" doesn't really mean anything to many users, and will likely be confusing. It seems like it would be more valuable with some copy that explains that it will update the versions from the VCS provider, and what that will do.

agjohnson · 2019-11-26T17:53:38Z

I like to avoid putting copy in UI, it clutters the UI and is often just going to be skipped by the user. I think a refresh icon button communicates more that "resync versions" and doesn't require us to explain our UI.

agjohnson

Changes look good, i noted some UI changes.

I think there will be a better UX here if we rely on a publictask or API call, similar to the import refresh call, and we can avoid communicating task progression through notification messages. This is a good first step for now, a good UX solution will add more complexity to get UI state correct.

readthedocs/projects/views/private.py

agjohnson · 2019-11-26T18:01:12Z

readthedocs/projects/views/private.py

+        else:
+            messages.error(
+                request,
+                _("There was an error when syncing project's versions"),


Same feedback here, but also: there isn't much the user can do with this error. Do we have any additional information we can display in the sync variable? What error does a failure with the sync task point at otherwise? Perhaps there is more guidance that we can give the user.

As for the message, it's better to speak to the user -- something closer to:

Suggested change

_("There was an error when syncing project's versions"),

_("There was a problem updating the project versions"),

@agjohnson Actually sync_versions function only calls a celery task sync_repository_task So, I don't know if we can show much more information from sync variable.

agjohnson · 2019-11-26T18:02:14Z

readthedocs/templates/projects/project_version_list.html

+                <li>
+                  <form method="post" action="{% url 'project_version_sync' project_slug=project.slug %}">
+                    {% csrf_token %}
+                    <input type="submit" value="{% trans 'Sync Versions' %}">


I think a refresh icon button probably communicates what we need here and doesn't require us to add copy to our UI.

I have added this one:

Another option might be:

Co-Authored-By: Anthony <[email protected]>

…into project-resync

stale · 2020-01-12T20:08:38Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

saadmk11 · 2020-01-13T13:42:33Z

I think this is still valid

stale · 2020-02-27T16:41:35Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

ericholscher · 2020-04-23T13:50:58Z

@agjohnson would be good for you to take a peek here. I think it's probably worth integrating into the new design.

humitos · 2022-02-22T16:14:11Z

@agjohnson what are the next actionable steps for this PR? Should we resolve conflicts and merge it so it's included in the new templates?

humitos · 2023-08-09T09:19:12Z

This PR works over the old templates, so I don't think it makes too much sense to try to recover it since we will need to migrate it to https://github.com/readthedocs/ext-theme anyways. There is some backend work we could recover once we implement this on ext-theme, tho. I'm closing this PR for now since we haven't had the time to prioritize it anyways.

Repository Re-sync View Added

935af08

saadmk11 requested review from ericholscher and a team September 26, 2019 13:49

tests fix

d5a3e6b

stsewd reviewed Sep 26, 2019

View reviewed changes

readthedocs/templates/projects/project_version_list.html Outdated Show resolved Hide resolved

UI Update

4bd6bf2

saadmk11 requested a review from stsewd September 26, 2019 18:16

stsewd approved these changes Sep 26, 2019

View reviewed changes

stsewd requested a review from a team September 26, 2019 18:43

Merge branch 'master' into project-resync

a6dc619

humitos reviewed Oct 9, 2019

View reviewed changes

saadmk11 and others added 3 commits October 9, 2019 19:09

Update readthedocs/projects/views/private.py

924c9c4

Co-Authored-By: Manuel Kaufmann <[email protected]>

Update readthedocs/templates/projects/project_version_list.html

e8bd71a

Co-Authored-By: Manuel Kaufmann <[email protected]>

Name and template update

c5344c5

saadmk11 requested a review from humitos October 9, 2019 13:46

saadmk11 closed this Oct 9, 2019

saadmk11 reopened this Oct 9, 2019

humitos approved these changes Oct 10, 2019

View reviewed changes

remove extra lines

26525ef

stale bot added the Status: stale Issue will be considered inactive soon label Nov 25, 2019

stsewd removed the Status: stale Issue will be considered inactive soon label Nov 25, 2019

stsewd requested a review from a team November 25, 2019 18:59

agjohnson requested changes Nov 26, 2019

View reviewed changes

saadmk11 and others added 5 commits November 29, 2019 01:07

Update readthedocs/projects/views/private.py

6b7af5b

Co-Authored-By: Anthony <[email protected]>

Update readthedocs/projects/views/private.py

b01c9dc

Co-Authored-By: Anthony <[email protected]>

Added refresh button

2f3d443

Merge branch 'project-resync' of github.com:saadmk11/readthedocs.org …

b1f80ad

…into project-resync

Merge branch 'master' into project-resync

1da019a

saadmk11 requested a review from agjohnson November 28, 2019 19:39

stale bot added the Status: stale Issue will be considered inactive soon label Jan 12, 2020

stale bot removed the Status: stale Issue will be considered inactive soon label Jan 13, 2020

stale bot added the Status: stale Issue will be considered inactive soon label Feb 27, 2020

stsewd added Accepted Accepted issue on our roadmap and removed Status: stale Issue will be considered inactive soon labels Feb 27, 2020

humitos closed this Aug 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Re-sync Repository from VCS #6210

Re-sync Repository from VCS #6210

saadmk11 commented Sep 26, 2019 •

edited by ericholscher

Loading

stsewd left a comment

saadmk11 commented Sep 26, 2019

humitos left a comment

humitos left a comment

stsewd commented Oct 10, 2019

humitos commented Oct 11, 2019

stsewd commented Oct 11, 2019

humitos commented Oct 11, 2019

stale bot commented Nov 25, 2019

ericholscher commented Nov 26, 2019

agjohnson commented Nov 26, 2019

agjohnson left a comment

agjohnson Nov 26, 2019

saadmk11 Nov 28, 2019

agjohnson Nov 26, 2019

saadmk11 Nov 28, 2019

saadmk11 Nov 28, 2019

stale bot commented Jan 12, 2020

saadmk11 commented Jan 13, 2020

stale bot commented Feb 27, 2020

ericholscher commented Apr 23, 2020

humitos commented Feb 22, 2022

humitos commented Aug 9, 2023

	_("There was an error when syncing project's versions"),
	_("There was a problem updating the project versions"),

Re-sync Repository from VCS #6210

Re-sync Repository from VCS #6210

Conversation

saadmk11 commented Sep 26, 2019 • edited by ericholscher Loading

stsewd left a comment

Choose a reason for hiding this comment

saadmk11 commented Sep 26, 2019

humitos left a comment

Choose a reason for hiding this comment

humitos left a comment

Choose a reason for hiding this comment

stsewd commented Oct 10, 2019

humitos commented Oct 11, 2019

stsewd commented Oct 11, 2019

humitos commented Oct 11, 2019

stale bot commented Nov 25, 2019

ericholscher commented Nov 26, 2019

agjohnson commented Nov 26, 2019

agjohnson left a comment

Choose a reason for hiding this comment

agjohnson Nov 26, 2019

Choose a reason for hiding this comment

saadmk11 Nov 28, 2019

Choose a reason for hiding this comment

agjohnson Nov 26, 2019

Choose a reason for hiding this comment

saadmk11 Nov 28, 2019

Choose a reason for hiding this comment

saadmk11 Nov 28, 2019

Choose a reason for hiding this comment

stale bot commented Jan 12, 2020

saadmk11 commented Jan 13, 2020

stale bot commented Feb 27, 2020

ericholscher commented Apr 23, 2020

humitos commented Feb 22, 2022

humitos commented Aug 9, 2023

saadmk11 commented Sep 26, 2019 •

edited by ericholscher

Loading