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

fix: Check if identifier does not contain of all digits. #6572

Merged

Conversation

prateekj117
Copy link
Member

Fixes #6423

Short description of what this resolves:

The public event page does not open in case the identifier of the event contains all digits.

Checklist

  • I have read the Contribution & Best practices Guide and my PR follows them.
  • My branch is up-to-date with the Upstream development branch.
  • The unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • All the functions created/modified in this PR contain relevant docstrings.

@codecov
Copy link

codecov bot commented Nov 4, 2019

Codecov Report

Merging #6572 into development will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #6572      +/-   ##
===============================================
+ Coverage        65.03%   65.03%   +<.01%     
===============================================
  Files              296      296              
  Lines            15248    15247       -1     
===============================================
  Hits              9916     9916              
+ Misses            5332     5331       -1
Impacted Files Coverage Δ
app/models/event.py 78.92% <100%> (+0.28%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bdcfb49...73e6046. Read the comment docs.

@prateekj117
Copy link
Member Author

@iamareebjamal As you told on Gitter that you have appended ref on the identifiers of the events which contain all digits, so we don't need to handle the old cases now, right?

@iamareebjamal
Copy link
Member

Not everyone who has open event server will have done it, so we need to handle it

Make a script which goes through events which have digit identifier and updates their identifier

So, that we can add in release note to just run python manage.py fix digit_identifier

@prateekj117
Copy link
Member Author

@iamareebjamal Done. Please review.

manage.py Outdated Show resolved Hide resolved
manage.py Outdated Show resolved Hide resolved
manage.py Outdated Show resolved Hide resolved
app/models/event.py Outdated Show resolved Hide resolved
@@ -37,6 +37,15 @@ def add_event_identifier():
save_to_db(event)


@manager.command
def fix_digit_identifier():
events = Event.query.filter(Event.identifier.op('~')('^[0-9\.]+$')).all()

Choose a reason for hiding this comment

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

invalid escape sequence '.'

@prateekj117
Copy link
Member Author

@iamareebjamal Please review.

app/models/event.py Outdated Show resolved Hide resolved
@prateekj117
Copy link
Member Author

@iamareebjamal Done.

iamareebjamal
iamareebjamal previously approved these changes Nov 6, 2019
@iamareebjamal
Copy link
Member

Add the command in CHANGELOG.md to be run for correcting the identifiers

@prateekj117
Copy link
Member Author

@iamareebjamal Done.

@iamareebjamal iamareebjamal merged commit 4921227 into fossasia:development Nov 6, 2019
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.

Event on public page fails to open
3 participants