-
Notifications
You must be signed in to change notification settings - Fork 87
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
Implemented the event search functionality . Fixes issue #516 #581
base: development
Are you sure you want to change the base?
Conversation
Reviewer's Guide by SourceryThis pull request implements the event search functionality using eventyay common URLs and fixes an issue related to assigning position secrets. The event search is implemented using a typeahead search bar and a new API endpoint. The position secret fix ensures that secrets are unique within an event. Updated class diagram for EventSearchViewclassDiagram
class EventSearchView {
+get(request)
}
class APIView {
}
EventSearchView --|> APIView
EventSearchView : -query
EventSearchView : -events
EventSearchView : -results
EventSearchView : +JsonResponse results
note for EventSearchView "Handles the search API endpoint"
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @suhailnadaf509 - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a debounce to the search input to reduce the number of API calls.
- The JavaScript code should be placed in a separate file and properly linked in the template.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
src/pretix/eventyay_common/static/eventyay-common/js/ui/dashboard.js
Outdated
Show resolved
Hide resolved
display: "name", | ||
source: eventSearch, | ||
templates: { | ||
suggestion: function (data) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (complexity): Consider using ES6 template literals to simplify the HTML construction and improve readability of the code.
Consider using ES6 template literals to simplify the HTML construction. For example, replace:
return (
'<div><a href="/common/event/' +
data.organizer +
"/" +
data.slug +
'/">' +
data.name +
"</a></div>"
);
with:
return `<div><a href="/common/event/${data.organizer}/${data.slug}/">${data.name}</a></div>`;
This refactoring reduces complexity and improves readability while preserving all functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated the dashboard.js file to use the ES6 template
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
…ard.js Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
I've made the following changes to implement the event search functionality that directs to eventyay common instead of eventyay tickets:
Closes #516
Summary by Sourcery
New Features: