PR#5007: Fixed forms' glitches. - pagure

pagure

#5007 Fixed forms' glitches.

Closed 3 years ago by mzch. Opened 3 years ago by mzch.

Unknown source master into master

Merge branch 'master' of ssh://pagure.io/forks/mzch/pagure into master

Koichi MATSUMOTO • 3 years ago

8d92660

fix AssertionError

Koichi MATSUMOTO • 3 years ago

beb1574

Fixed forms' glitches.

Koichi MATSUMOTO • 3 years ago

4fcf88e

Fixed forms' glitches.

Koichi MATSUMOTO • 3 years ago

8cc752c

pagure/api/issue.py

file modified

+1 -1

		`@@ -1593,7 +1593,7 @@`
		`weeks_range = flask.request.args.get("weeks_range") or 53`
		`try:`
		`weeks_range = int(weeks_range)`
		`- except:`
		`+ except Exception:`
		`weeks_range = 53`

		`repo = _get_repo(repo, username, namespace)`

pagure/forms.py

file modified

+25 -25

		`@@ -136,7 +136,7 @@`
		`""" Form to edit the description of a project. """`

		`description = wtforms.StringField(`
		`- 'Description <span class="error">*</span>',`
		`+ 'Description',`
		`[wtforms.validators.DataRequired()],`
		`)`
		`url = wtforms.StringField(`
		`@@ -169,7 +169,7 @@`
		`class ProjectForm(ProjectFormSimplified):`
		`""" Form to create or edit project. """`

		`- name = wtforms.StringField('Project name <span class="error">*</span>')`
		`+ name = wtforms.StringField('Project name')`
		`mirrored_from = wtforms.StringField(`
		`"Mirror from URL",`
		`[`
		`@@ -252,11 +252,11 @@`
		`""" Form to create or edit an issue. """`

		`title = wtforms.StringField(`
		`- 'Title<span class="error">*</span>',`
		`+ 'Title',`
		`[wtforms.validators.DataRequired()],`
		`)`
		`issue_content = wtforms.TextAreaField(`
		`- 'Content<span class="error">*</span>',`
		`+ 'Content',`
		`[wtforms.validators.DataRequired()],`
		`)`
		`private = wtforms.BooleanField(`
		`@@ -317,7 +317,7 @@`
		`""" Form to create a pull request. """`

		`title = wtforms.StringField(`
		`- 'Title<span class="error">*</span>',`
		`+ 'Title',`
		`[wtforms.validators.DataRequired()],`
		`)`
		`initial_comment = wtforms.TextAreaField(`
		`@@ -334,18 +334,18 @@`
		`""" Form to create a remote pull request. """`

		`git_repo = wtforms.StringField(`
		`- 'Git repo address<span class="error">*</span>',`
		`+ 'Git repo address',`
		`[`
		`wtforms.validators.DataRequired(),`
		`wtforms.validators.Regexp(urlpattern, flags=re.IGNORECASE),`
		`],`
		`)`
		`branch_from = wtforms.StringField(`
		`- 'Git branch<span class="error">*</span>',`
		`+ 'Git branch',`
		`[wtforms.validators.DataRequired()],`
		`)`
		`branch_to = wtforms.StringField(`
		`- 'Git branch to merge in<span class="error">*</span>',`
		`+ 'Git branch to merge in',`
		`[wtforms.validators.DataRequired()],`
		`)`

		`@@ -554,7 +554,7 @@`
		`requestid = wtforms.HiddenField("requestid")`
		`tree_id = wtforms.HiddenField("treeid")`
		`comment = wtforms.TextAreaField(`
		`- 'Comment<span class="error">*</span>',`
		`+ 'Comment',`
		`[wtforms.validators.DataRequired()],`
		`)`

		`@@ -605,7 +605,7 @@`
		`""" Form to add a SSH key to a user. """`

		`ssh_key = wtforms.StringField(`
		`- 'SSH Key <span class="error">*</span>',`
		`+ 'SSH Key',`
		`[wtforms.validators.DataRequired()]`
		`# TODO: Add an ssh key validator?`
		`)`
		`@@ -625,15 +625,15 @@`
		`""" Form to add a user to a project. """`

		`user = wtforms.StringField(`
		`- 'Username <span class="error">*</span>',`
		`+ 'Username',`
		`[wtforms.validators.DataRequired()],`
		`)`
		`access = wtforms.StringField(`
		`- 'Access Level <span class="error">*</span>',`
		`+ 'Access Level',`
		`[wtforms.validators.DataRequired()],`
		`)`
		`branches = wtforms.StringField(`
		`- 'Git branches <span class="error">*</span>',`
		`+ 'Git branches',`
		`[wtforms.validators.Optional()],`
		`)`

		`@@ -642,7 +642,7 @@`
		`""" Form to add a user to a pagure group. """`

		`user = wtforms.StringField(`
		`- 'Username <span class="error">*</span>',`
		`+ 'Username',`
		`[wtforms.validators.DataRequired()],`
		`)`

		`@@ -651,7 +651,7 @@`
		`""" Form to assign an user to an issue. """`

		`assignee = wtforms.StringField(`
		`- 'Assignee <span class="error">*</span>',`
		`+ 'Assignee',`
		`[wtforms.validators.Optional()],`
		`)`

		`@@ -660,18 +660,18 @@`
		`""" Form to add a group to a project. """`

		`group = wtforms.StringField(`
		`- 'Group <span class="error">*</span>',`
		`+ 'Group',`
		`[`
		`wtforms.validators.DataRequired(),`
		`wtforms.validators.Regexp(STRICT_REGEX, flags=re.IGNORECASE),`
		`],`
		`)`
		`access = wtforms.StringField(`
		`- 'Access Level <span class="error">*</span>',`
		`+ 'Access Level',`
		`[wtforms.validators.DataRequired()],`
		`)`
		`branches = wtforms.StringField(`
		`- 'Git branches <span class="error">*</span>',`
		`+ 'Git branches',`
		`[wtforms.validators.Optional()],`
		`)`

		`@@ -691,7 +691,7 @@`
		`choices=[("user", "User"), ("group", "Group")],`
		`)`
		`name = wtforms.StringField(`
		`- 'User- or Groupname <span class="error">*</span>',`
		`+ 'User- or Groupname',`
		`[wtforms.validators.DataRequired()],`
		`)`
		`acl = wtforms.SelectField(`
		`@@ -773,7 +773,7 @@`
		`""" Form to ask for a password change. """`

		`group_name = wtforms.StringField(`
		`- 'Group name <span class="error">*</span>',`
		`+ 'Group name',`
		`[`
		`wtforms.validators.DataRequired(),`
		`wtforms.validators.Length(max=255),`
		`@@ -866,7 +866,7 @@`
		`"""`

		`update_comment = wtforms.TextAreaField(`
		`- 'Comment<span class="error">*</span>',`
		`+ 'Comment ',`
		`[wtforms.validators.DataRequired()],`
		`)`

		`@@ -890,7 +890,7 @@`
		`"""`

		`report_name = wtforms.TextAreaField(`
		`- 'Report name<span class="error">*</span>',`
		`+ 'Report name',`
		`[wtforms.validators.DataRequired()],`
		`)`

		`@@ -900,12 +900,12 @@`
		`"""`

		`issue_notifs = wtforms.TextAreaField(`
		`- 'Public issue notification<span class="error">*</span>',`
		`+ 'Public issue notification',`
		`[wtforms.validators.optional(), MultipleEmail()],`
		`)`

		`pr_notifs = wtforms.TextAreaField(`
		`- 'Public PR notification<span class="error">*</span>',`
		`+ 'Public PR notification',`
		`[wtforms.validators.optional(), MultipleEmail()],`
		`)`

		`@@ -953,7 +953,7 @@`
		`""" Form to create a new git tag. """`

		`tagname = wtforms.StringField(`
		`- 'Name of the tag<span class="error">*</span>',`
		`+ 'Name of the tag',`
		`[wtforms.validators.DataRequired()],`
		`)`
		`commit_hash = wtforms.StringField(`

pagure/hooks/mail.py

file modified

+1 -1

		`@@ -128,7 +128,7 @@`
		`proc = subprocess.Popen(`
		`[hook_file], cwd=repodir, stdin=subprocess.PIPE`
		`)`
		`- proc.communicate(stdin)`
		`+ proc.communicate(stdin.encode())`
		`ecode = proc.wait()`
		`if ecode != 0:`
		`print("git_multimail failed")`

pagure/login_forms.py

file modified

+12 -12

		`@@ -43,7 +43,7 @@`
		`""" Form to ask for a password change. """`

		`username = wtforms.StringField(`
		`- 'username <span class="error">*</span>',`
		`+ 'username',`
		`[wtforms.validators.DataRequired()],`
		`)`

		`@@ -52,11 +52,11 @@`
		`""" Form to reset one's password in the local database. """`

		`password = wtforms.PasswordField(`
		`- 'Password <span class="error">*</span>',`
		`+ 'Password',`
		`[wtforms.validators.DataRequired()],`
		`)`
		`confirm_password = wtforms.PasswordField(`
		`- 'Confirm password <span class="error">*</span>',`
		`+ 'Confirm password',`
		`[wtforms.validators.DataRequired(), same_password],`
		`)`

		`@@ -65,11 +65,11 @@`
		`""" Form to login via the local database. """`

		`username = wtforms.StringField(`
		`- 'username <span class="error">*</span>',`
		`+ 'username',`
		`[wtforms.validators.DataRequired()],`
		`)`
		`password = wtforms.PasswordField(`
		`- 'Password <span class="error">*</span>',`
		`+ 'Password',`
		`[wtforms.validators.DataRequired()],`
		`)`

		`@@ -78,22 +78,22 @@`
		`""" Form to add a new user to the local database. """`

		`user = wtforms.StringField(`
		`- 'username <span class="error">*</span>',`
		`+ 'username',`
		`[wtforms.validators.DataRequired()],`
		`)`
		`fullname = wtforms.StringField(`
		`"Full name", [wtforms.validators.Optional()]`
		`)`
		`email_address = wtforms.StringField(`
		`- 'Email address <span class="error">*</span>',`
		`+ 'Email address',`
		`[wtforms.validators.DataRequired(), wtforms.validators.Email()],`
		`)`
		`password = wtforms.PasswordField(`
		`- 'Password <span class="error">*</span>',`
		`+ 'Password',`
		`[wtforms.validators.DataRequired()],`
		`)`
		`confirm_password = wtforms.PasswordField(`
		`- 'Confirm password <span class="error">*</span>',`
		`+ 'Confirm password',`
		`[wtforms.validators.DataRequired(), same_password],`
		`)`

		`@@ -102,14 +102,14 @@`
		`""" Form to reset one's password in the local database. """`

		`old_password = wtforms.PasswordField(`
		`- 'Old Password <span class="error">*</span>',`
		`+ 'Old Password',`
		`[wtforms.validators.DataRequired()],`
		`)`
		`password = wtforms.PasswordField(`
		`- 'Password <span class="error">*</span>',`
		`+ 'Password',`
		`[wtforms.validators.DataRequired()],`
		`)`
		`confirm_password = wtforms.PasswordField(`
		`- 'Confirm password <span class="error">*</span>',`
		`+ 'Confirm password',`
		`[wtforms.validators.DataRequired(), same_password],`
		`)`

pagure/templates/_formhelper.html

file modified

+1 -1

		`@@ -31,7 +31,7 @@`
		`{% endif %}`
		`{% else %}`
		`<div>`
		`- <strong>{{ field.label }}</strong>`
		`+ <strong>{{ field.label }} {% if field.errors %} <span class="error">*</span> {% endif %}</strong>`
		`{% if rightlink %}`
		`<div class="float-right"><a href="{{rightlink['url']}}">{{rightlink['text']}}</a>`
		`{% endif %}`

tests/test_pagure_flask_ui_app.py

file modified

+2 -2

		`@@ -2645,7 +2645,7 @@`
		`output_text = output.get_data(as_text=True)`
		`self.assertIn(`
		`'<strong><label for="mirrored_from">Mirror from URL'`
		`- "</label></strong>",`
		`+ "</label> </strong>",`
		`output_text,`
		`)`

		`@@ -2683,7 +2683,7 @@`
		`output_text = output.get_data(as_text=True)`
		`self.assertNotIn(`
		`'<strong><label for="mirrored_from">Mirror from URL'`
		`- "</label></strong>",`
		`+ "</label> </strong>",`
		`output_text,`
		`)`

tests/test_pagure_flask_ui_groups.py

file modified

+2 -2

		`@@ -197,7 +197,7 @@`
		`)`
		`self.assertIn(`
		`'<strong><label for="description">Description'`
		`- "</label></strong>",`
		`+ "</label> </strong>",`
		`output.get_data(as_text=True),`
		`)`

		`@@ -228,7 +228,7 @@`
		`)`
		`self.assertIn(`
		`'<strong><label for="description">Description'`
		`- "</label></strong>",`
		`+ "</label> </strong>",`
		`output.get_data(as_text=True),`
		`)`

mzch commented 3 years ago

Fixed forms' glitches
- Remove <span class="errors">*</span> from forms.py and login_forms.py
- Add <span class="errors">*</span> to templates/_formhelper.html

pingou commented 3 years ago

I like the idea and the change, though I suspect a few tests need to be adjusted for this :)

rebased onto 4fcf88e

3 years ago

mzch commented 3 years ago

Yes, I also agree to have tests, but my instance is running without problem as of now.

pingou commented 3 years ago

Looks like there are three tests failing:

tests/test_pagure_flask_ui_groups.py:201: AssertionError
tests/test_pagure_flask_ui_app.py:2649: AssertionError
tests/test_style.py:105: AssertionError

There is another one at tests/test_pagure_flask_ui_login.py:1136: but that seems weird

mzch commented 3 years ago

would reformat ../pagure/runserver.py
Oh no! 💥 💔 💥
91 files would be reformatted, 340 files would be left unchanged.

It seems many files hit.

rebased onto 8cc752c

3 years ago

pingou commented 3 years ago

Check the version of black you're using, we're still using black == 19.10b0; only

pingou commented 3 years ago

I also run black simply using black -l 79 tests/test_*.py pagure

pingou commented 3 years ago

Also, please use git rebase instead of git merge, we try to keep the history linear and avoid merge commits as much as possible :)

mzch commented 3 years ago

I ran black -l 79 tests/test_*.py pagure, then I got:

pagure@lb:~/pagure$ black -l 79 tests/test_*.py pagure
reformatted /srv/pagure/pagure/pagure/hooks/irc.py
reformatted /srv/pagure/pagure/pagure/forms.py
reformatted /srv/pagure/pagure/pagure/login_forms.py
All done! ✨ 🍰 ✨
3 files reformatted, 194 files left unchanged.

with no error.

pingou commented 3 years ago

Did you delete your fork?

mzch commented 3 years ago

Yes. please close this PR.

Pull-Request has been closed by mzch

3 years ago