Feat: Accepted Tokens API Endpoint #10
Closed
DotworldX wants to merge 2 commits intoTrajectfi:mainfrom
Closed
Conversation
## Description <!-- Provide a clear and concise description of your changes --> ## Related Issue <!-- Link to the related issue (if applicable) --> Fixes #[Issue Number] ## Type of Change <!-- Please mark the relevant option(s) --> - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] Performance improvement - [ ] Code refactoring - [ ] Documentation update - [ ] Test coverage improvement - [ ] CI/CD related changes ## How Has This Been Tested? <!-- Describe the tests you ran to verify your changes --> - [ ] Unit tests - [ ] Integration tests - [ ] Manual testing <!-- Provide details about test environment if applicable --> ## Python Dependencies <!-- List any new Python dependencies added --> ``` # New dependencies: # example_package==1.0.0 ``` ## Starknet-Specific Considerations <!-- Any Starknet-specific concerns or optimizations --> ## Checklist - [ ] My code follows the project's style guidelines - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] Any dependent changes have been merged and published in downstream modules ## Security Considerations <!-- Describe any security implications of your changes --> - [ ] This PR introduces no security concerns - [ ] This PR modifies authentication/authorization - [ ] This PR handles sensitive data - [ ] This PR modifies financial transaction logic ## Performance Impact <!-- Describe any performance impact of your changes --> - [ ] This PR has no significant performance impact - [ ] This PR improves performance - [ ] This PR may impact performance negatively (explain why it's necessary) ## Additional Context <!-- Add any other context about the PR here -->
jaykayudo
suggested changes
Apr 4, 2025
Contributor
jaykayudo
left a comment
jaykayudo
left a comment
There was a problem hiding this comment.
Hello @DotworldX
Your pull request includes the key components required to complete this task, but there are a few changes that need to be made. Please review the requested changes and make the necessary adjustments.
|
|
||
| from django.core.asgi import get_asgi_application | ||
|
|
||
| sys.path.append(os.path.abspath(os.path.join(os.path.dirname(__file__), ".."))) |
Contributor
There was a problem hiding this comment.
What is this for?
I don't think it is needed
Comment on lines
+166
to
+169
| INSTALLED_APPS = [ | ||
| "core.tests", # Register the models path | ||
| "rest_framework", | ||
| ] |
Contributor
There was a problem hiding this comment.
There already is an INSTALLED_APPS variable containing all the necessary variable to run the app. This file is out of scope for your task.
Comment on lines
+28
to
+45
| # Serializer | ||
| class AcceptedTokenSerializer(serializers.ModelSerializer): | ||
| listing_count = serializers.SerializerMethodField() | ||
|
|
||
| class Meta: | ||
| model = AcceptedToken | ||
| fields = ["contract_address", "name", "listing_count"] | ||
|
|
||
| def get_listing_count(self, obj): | ||
| return obj.listing_count | ||
|
|
||
|
|
||
| # View | ||
| class AcceptedTokenListView(generics.ListAPIView): | ||
| queryset = AcceptedToken.objects.annotate(listing_count=Count("listing")) | ||
| serializer_class = AcceptedTokenSerializer | ||
|
|
||
|
|
Contributor
There was a problem hiding this comment.
These should have been implemented in the serializers.py and views.py files respectively.
Also the AcceptedToken model has no field called listing neither is there any field that references it that is called listing.
jaykayudo
changed the title
Author
|
sorry I am just gettinng back to you. Can i still work on it? |
Contributor
|
Yes. It’s still yours. |
Contributor
|
Hello @DotworldX |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Related Issue
Fixes #[Issue Number]
Type of Change
How Has This Been Tested?
Python Dependencies
Starknet-Specific Considerations
Checklist
Security Considerations
Performance Impact
Additional Context