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

Project Settings / Delete Project #2821

Merged
merged 7 commits into from
Jan 29, 2018
Merged

Project Settings / Delete Project #2821

merged 7 commits into from
Jan 29, 2018

Conversation

di
Copy link
Member

@di di commented Jan 22, 2018

Fixes #2805. This moves the releases to their own route, and stubs out an "options" page with a basic form for project deletion.

(cc @nlhkabu)

@nlhkabu
Copy link
Contributor

nlhkabu commented Jan 22, 2018

Awesome, thanks @di!
I will pull it down and take a look :)

@nlhkabu
Copy link
Contributor

nlhkabu commented Jan 23, 2018

Hi @di, I've pushed up some changes in light of the new structure.

I've noticed that I'm getting an error on the collaborators page:

screenshot from 2018-01-23 06-57-31

Could you please take a look.

This is almost ready to merge on my side - I just need to look at the mobile UI.

@di
Copy link
Member Author

di commented Jan 23, 2018

@nlhkabu Sorry about that, fixed now!

@nlhkabu
Copy link
Contributor

nlhkabu commented Jan 23, 2018

Thanks @di - I'll fix the rest of the UI tomorrow, so hopefully we can merge :)

@nlhkabu
Copy link
Contributor

nlhkabu commented Jan 26, 2018

This page is now done from a style POV on both mobile and desktop:

screenshot from 2018-01-26 07-33-23
fireshot capture 9 - manage opstacle setting_ - http___localhost_manage_project_opstacle_settings_
screenshot from 2018-01-26 07-33-05

Still a bit ugly on mobile:

  • Projects page (list of projects)
  • Releases table
  • Collaborators table

I suggest we merge this PR and I can open a new one to address these, as well as #2838

What do you think @di?

@nlhkabu
Copy link
Contributor

nlhkabu commented Jan 26, 2018

Ok, rebased with master, so we should be ready to go 😁

One thing I noticed in the templates:

<div class="package-snippet__buttons">
  <a href="{{ request.route_path('manage.project.releases', project_name=project.normalized_name) }}" class="button button--primary">Edit</a>
  <a href="{{ request.route_path('packaging.project', name=project.normalized_name) }}" class="button">View</a>
</div>

Is there a good reason why one is project_name=blah while the other is project=blah ?

@di
Copy link
Member Author

di commented Jan 26, 2018

I suggest we merge this PR and I can open a new one to address these, as well as #2838

Sounds fine to me!

Is there a good reason why one is project_name=blah while the other is project=blah ?

Yeah, I changed all of the manage routes to take a project_name instead so I can reuse the confirm_project and delete_project utility functions that we already have on the admin side of things. All the admin routes took a project_name instead of just name and I figured that the former was less ambiguous.

It's probably worth updating all the packaging routes to take a project_name instead as well, but since it's not really necessary for this PR, I left it for later.

@di
Copy link
Member Author

di commented Jan 26, 2018

I made a small tweak here to match the new "Edit Project" button, which is that "Project Settings" is the first item in the menu, and the default when you press the "Edit" button on the list of projects.

We should fix #2418 (via #2849) before merging this, so that deleting a project purges the cache.

@nlhkabu
Copy link
Contributor

nlhkabu commented Jan 27, 2018

Hi @di - I agree that the 'edit' button should go to the same place, but I'm not sure that I agree that the 'settings' page is the best landing page?

At the moment the only thing on there is to delete a project, which we want to "hide" as much as possible in the UI.

What is the most common action a project owner/maintainer will want to do? Look at / edit their releases, manage their collaborators, or change the settings? Maybe we can get this information from the current site analytics?

@di
Copy link
Member Author

di commented Jan 29, 2018

@nlhkabu Great point. I changed this back to make the "releases" page the default, and we can reevaluate the ordering later if need be.

@di di merged commit 0c46c65 into master Jan 29, 2018
@di di deleted the manage-project-settings branch January 29, 2018 17:02
@nlhkabu
Copy link
Contributor

nlhkabu commented Jan 29, 2018

giphy-downsized 3

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