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

New notifications #1356

Merged
merged 7 commits into from
Jul 13, 2024
Merged

New notifications #1356

merged 7 commits into from
Jul 13, 2024

Conversation

KoalaSat
Copy link
Member

@KoalaSat KoalaSat commented Jun 28, 2024

What does this PR do?

Includes the necessary changes to be compatible with the existing notifications on web

Checklist before merging

  • Install pre-commit and initialize it: pip install pre-commit, then pre-commit install. Pre-commit installs git hooks that automatically check the codebase. If pre-commit fails when you commit your changes, please fix the problems it points out.

# self.assertEqual(
# notifications_data[0]["title"],
# f"⚖️ Hey {data['taker_nick']}, you lost the dispute on your order with ID {str(trade.order_id)}."
# )
Copy link
Member Author

Choose a reason for hiding this comment

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

Since the dispute resolution is an admin action, I'm not quite sure how to do it, ideas @Reckless-Satoshi ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder whether we could directly call

def maker_wins(self, request, queryset):
... Looks difficult. Maybe we should explore how the request sent from the admin panel looks when solving a dispute and try to imitate it here?

Copy link
Member Author

Choose a reason for hiding this comment

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

And fake the admin request with credentials? I'll take a look but for this kind of solutions they are always tricky

@KoalaSat KoalaSat force-pushed the new-notifications branch 3 times, most recently from 4d11c24 to a9f1696 Compare June 30, 2024 21:29
@KoalaSat KoalaSat force-pushed the new-notifications branch from a9f1696 to adbaa60 Compare July 9, 2024 19:39
@KoalaSat KoalaSat mentioned this pull request Jul 12, 2024
1 task
@KoalaSat
Copy link
Member Author

Merging this changes as agreed with @Reckless-Satoshi to figure out in other PR what to do with admin actions on tests

@KoalaSat
Copy link
Member Author

I don´t understand why ruff is complaining on lines doesn´t even exist anymore 🙃

@KoalaSat KoalaSat merged commit 886956c into main Jul 13, 2024
1 check failed
@KoalaSat KoalaSat deleted the new-notifications branch July 13, 2024 11:37
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.

2 participants