Skip to content

Added new and edit for organization#222

Open
shreyas-satish wants to merge 5 commits intomainfrom
orgview
Open

Added new and edit for organization#222
shreyas-satish wants to merge 5 commits intomainfrom
orgview

Conversation

@shreyas-satish
Copy link
Copy Markdown
Contributor

No description provided.

Comment thread boxoffice/forms/org.py Outdated


DEFAULT_ORG_DETAILS = {
u'access_token': buid(),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is declaring a fixed access token as a global.

Comment thread boxoffice/forms/org.py Outdated

class OrgForm(forms.Form):
contact_email = forms.EmailField(__("Contact email"),
validators=[forms.validators.DataRequired(__("Please enter an email address")), forms.validators.Length(min=5, max=80)])
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Use an email validator instead of a length validator.

Comment thread boxoffice/forms/utils.py Outdated
# so assign a default value of `{}`
if not data or data == 'null':
return json.dumps({})
return data
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So it doesn't format JSON if the request is not a GET? That doesn't make sense.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The formatting with indents is only to make it easy for the admin to edit it. simplejson fails when the formatted version is supplied via populate_obj.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I mean the "return data" bit. It's returning the Python data object if the request is not a GET? Why?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's because json.dumps(data) will result in a JSON string, which errors out when I attempt to store it in the database. Since it expects a dict, I just return the data dict.

Comment thread boxoffice/views/org.py
return render_message(
title=_(u"No organizations available"),
message=_(u"To setup Boxoffice for an organization, you must be the owner of the organization."))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Use the logic used in Funnel's /new handler. This code looks fragile.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Damn, then that needs to be tossed as well. We can live with this for now, knowing that both places will need to be changed after hasgeek/lastuser#232.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented May 16, 2022

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
0 out of 2 committers have signed the CLA.

❌ Bibhas
❌ shreyas-satish


Bibhas seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

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.

3 participants