Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

Implement project (Team) closing #4287

Merged
merged 10 commits into from
Jan 12, 2017
Merged

Implement project (Team) closing #4287

merged 10 commits into from
Jan 12, 2017

Conversation

chadwhitacre
Copy link
Contributor

@chadwhitacre chadwhitacre commented Jan 11, 2017

See #3602.

  • set is_closed
  • clear payment instructions (right?) Wrong. Leave the data for now.
  • clear takes (right?) Wrong. Leave the data for now.
  • remove from payday
  • remove from homepage
  • remove from search
  • are we showing 404? No: 410.
  • can you reopen a team? No.
  • UI to close team
  • close teams automatically (with warning?) during participant closing?
  • are we freeing up the name somehow? No, not now.

@chadwhitacre chadwhitacre mentioned this pull request Jan 11, 2017
5 tasks
@chadwhitacre chadwhitacre added this to the Finish Project Basics milestone Jan 11, 2017
@chadwhitacre
Copy link
Contributor Author

Interesting. We already have an is_closed field. :-)

@chadwhitacre
Copy link
Contributor Author

We even use it (etc.)!

@chadwhitacre
Copy link
Contributor Author

I don't understand how the edit form is wired.

@chadwhitacre
Copy link
Contributor Author

We're using global vars and FormData,

@chadwhitacre
Copy link
Contributor Author

We're probably using FormData to account for the file upload.

@chadwhitacre
Copy link
Contributor Author

Ready for review, @mattbk @nobodxbodon et al.

@mattbk
Copy link
Contributor

mattbk commented Jan 11, 2017

Closing my own or another project as admin:

Traceback (most recent call last):
  File "/home/matt/Documents/gratipay/gratipay.com/env/lib/python2.7/site-packages/algorithm.py", line 288, in run
    new_state = function(**deps.as_kwargs)
  File "/home/matt/Documents/gratipay/gratipay.com/env/lib/python2.7/site-packages/aspen/algorithms/website.py", line 115, in get_response_for_resource
    return {'response': resource.respond(state)}
  File "/home/matt/Documents/gratipay/gratipay.com/env/lib/python2.7/site-packages/aspen/http/resource.py", line 59, in respond
    content_type, body = super(Dynamic, self).respond(accept, state)
  File "/home/matt/Documents/gratipay/gratipay.com/env/lib/python2.7/site-packages/aspen/simplates/__init__.py", line 169, in respond
    exec(self.pages[1], context)
  File "/home/matt/Documents/gratipay/gratipay.com/www/~/%username/index.html.spt", line 16, in <module>
    statement, stmt_lang = participant.get_statement(request.accept_langs)
  File "/home/matt/Documents/gratipay/gratipay.com/gratipay/models/participant/__init__.py", line 204, in get_statement
    """, dict(id=self.id, langs=langs), default=(None, None, None))
  File "/home/matt/Documents/gratipay/gratipay.com/env/lib/python2.7/site-packages/postgres/__init__.py", line 482, in one
    return cursor.one(sql, parameters, default)
  File "/home/matt/Documents/gratipay/gratipay.com/env/lib/python2.7/site-packages/postgres/cursors.py", line 105, in one
    out = self._some(sql, parameters, lo=0, hi=1)
  File "/home/matt/Documents/gratipay/gratipay.com/env/lib/python2.7/site-packages/postgres/cursors.py", line 127, in _some
    self.execute(sql, parameters)
  File "/home/matt/Documents/gratipay/gratipay.com/env/lib/python2.7/site-packages/psycopg2/extras.py", line 288, in execute
    return super(NamedTupleCursor, self).execute(query, vars)
ProgrammingError: column "content_scrubbed" does not exist
LINE 2:             SELECT content, content_scrubbed, lang

@mattbk
Copy link
Contributor

mattbk commented Jan 11, 2017

Will make clean env and try again.

@mattbk
Copy link
Contributor

mattbk commented Jan 11, 2017

But if I refresh, it says "already closed."

@chadwhitacre
Copy link
Contributor Author

column "content_scrubbed" does not exist

That suggests that you needed to make schema to pick up the new column from #4266.

@chadwhitacre
Copy link
Contributor Author

But if I refresh, it says "already closed."

Okay, so the close succeeded and the content_scrubbed error came afterwards?

@mattbk
Copy link
Contributor

mattbk commented Jan 11, 2017

Okay, so the close succeeded and the content_scrubbed error came afterwards?

Clicking "close" gave me the error, but the close succeeded. Getting back up to speed.

@mattbk
Copy link
Contributor

mattbk commented Jan 11, 2017

Looks good from here!

@mattbk
Copy link
Contributor

mattbk commented Jan 11, 2017

Closed project is still listed on and accessible from ~user's profile.

raise Response(403, _("You are not authorized to access this page."))

if team.is_closed:
raise Response(403, _("Already closed."))
Copy link
Contributor

Choose a reason for hiding this comment

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

How about disable "close" button instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this was quicker to put together. We can enhance (add project reopening?) once the basic functionality is there.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good. BTW after your latest commit (hide closed team), in theory user should not reach here, as they won't see the already closed teams?

Copy link
Contributor

Choose a reason for hiding this comment

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

If you try to close again by URL for some reason, you would get this error. All my latest commit did was hide the link to the project from the project owner's profile.

@mattbk
Copy link
Contributor

mattbk commented Jan 12, 2017

I hope to check through this again tonight and try to fix anything I find.

@mattbk
Copy link
Contributor

mattbk commented Jan 12, 2017

Closed project is still listed on and accessible from ~user's profile.

Fixxored in 6b191f0

I like it!

Better match what we've already got.
@chadwhitacre
Copy link
Contributor Author

I added 36948c0 to improve (and test) our behavior around listing projects on owner profiles.

How does that look, @mattbk @nobodxbodon? Are we ready to merge?

@mattbk
Copy link
Contributor

mattbk commented Jan 12, 2017

Good to me.

@chadwhitacre
Copy link
Contributor Author

You good, @nobodxbodon?

@nobodxbodon
Copy link
Contributor

@whit537 sorry I'm not sure I can review your latest commit 36948c0 today. Please go ahread and merge if LG to both of you?

BTW I feel the trickiness when a PR involves multiple peoples' commits. Ideally a third person who doesn't have commits should merge?

@mattbk
Copy link
Contributor

mattbk commented Jan 12, 2017

If it makes you feel better, @whit537's commit overwrote mine 😉.

@chadwhitacre
Copy link
Contributor Author

chadwhitacre commented Jan 12, 2017

It improved on yours. :-P

@mattbk The merge is yours, I think.

@mattbk mattbk merged commit e10a405 into master Jan 12, 2017
@chadwhitacre
Copy link
Contributor Author

So is the delete branch? 👁️

@mattbk mattbk deleted the project-closing branch January 12, 2017 19:58
@mattbk
Copy link
Contributor

mattbk commented Jan 12, 2017

😝

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants