Feat: Add recently registered orgs to registration page #4833
Conversation
WalkthroughAdds Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant View as RegisterOrganizationView.get
participant DB as Organization Model
participant Template as register_organization.html
Browser->>View: GET /organizations/register/
View->>DB: Query active Organizations (order by created desc, limit 5)
DB-->>View: recent_organizations list
View->>Template: render(context including recent_organizations, form, messages)
Template-->>Browser: HTML with form + recently registered grid (if provided) + messages
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Knowledge base: Disabled due to 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (3)
🧰 Additional context used🪛 Ruff (0.14.5)website/test_feed.py17-17: Possible hardcoded password assigned to argument: "password" (S106) 21-21: Possible hardcoded password assigned to argument: "password" (S106) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
bot
added
files-changed: 3
github-project-automation
bot
moved this from Backlog
to Ready
in 📌 OWASP BLT Project Board
Satish-Medar
force-pushed
the
feat/add-recent-orgs-v2
branch
from
8a262c2 to
f7a8da2
Compare
|
👋 Hi @Satish-Medar! This pull request needs a peer review before it can be merged. Please request a review from a team member who is not:
Once a valid peer review is submitted, this check will pass automatically. Thank you! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
website/templates/organization/register_organization.html (1)
220-249: LGTM! Consider displaying organization logos.The recently registered organizations section is well-implemented with proper conditional rendering and responsive grid layout. The generic building icon provides consistency.
As an optional enhancement, consider displaying each organization's logo if available, falling back to the generic icon when no logo exists.
If you'd like to display organization logos, here's a suggested approach:
- <div class="flex-shrink-0 flex items-center justify-center w-16 h-16 bg-gray-200 rounded-l-md"> - <svg class="h-8 w-8 text-gray-600" - xmlns="http://www.w3.org/2000/svg" - fill="none" - viewBox="0 0 24 24" - stroke-width="1.5" - stroke="currentColor"> - <path stroke-linecap="round" stroke-linejoin="round" d="M3.75 21h16.5M4.5 3h15M5.25 3v18m13.5-18v18M8.25 6h7.5m-7.5 3h7.5m-7.5 3h7.5m-7.5 3h7.5m-7.5 3h7.5" /> - </svg> - </div> + <div class="flex-shrink-0 flex items-center justify-center w-16 h-16 bg-gray-200 rounded-l-md overflow-hidden"> + {% if org.logo %} + <img src="{{ org.logo.url }}" alt="{{ org.name }} logo" class="w-full h-full object-cover"> + {% else %} + <svg class="h-8 w-8 text-gray-600" + xmlns="http://www.w3.org/2000/svg" + fill="none" + viewBox="0 0 24 24" + stroke-width="1.5" + stroke="currentColor"> + <path stroke-linecap="round" stroke-linejoin="round" d="M3.75 21h16.5M4.5 3h15M5.25 3v18m13.5-18v18M8.25 6h7.5m-7.5 3h7.5m-7.5 3h7.5m-7.5 3h7.5m-7.5 3h7.5" /> + </svg> + {% endif %} + </div>website/views/company.py (1)
136-138: Optimize query to fetch only required fields.The query fetches all fields from the Organization model, but the template only uses
name,slug(viaget_absolute_url), andcreated. Consider using.only()to fetch only the required fields for better performance.Apply this diff to optimize the query:
- recent_organizations = Organization.objects.filter(is_active=True).order_by("-created")[:5] + recent_organizations = Organization.objects.filter(is_active=True).only("id", "name", "slug", "created").order_by("-created")[:5]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (3)
website/models.py(1 hunks)website/templates/organization/register_organization.html(1 hunks)website/views/company.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run Tests
- GitHub Check: docker-test
🔇 Additional comments (1)
website/models.py (1)
233-234: URL route verified and correctly configured.The
organization_detailURL pattern exists atblt/urls.py:1063and is properly configured with a slug parameter that matches the kwargs passed inget_absolute_url().
github-actions
bot
added
unresolved-conversations: 1
Satish-Medar
force-pushed
the
feat/add-recent-orgs-v2
branch
from
69540be to
9339236
Compare
github-actions
bot
added
unresolved-conversations: 0
Satish-Medar
force-pushed
the
feat/add-recent-orgs-v2
branch
from
de2d7d2 to
dca6c57
Compare
github-actions
bot
added
files-changed: 6
Satish-Medar
force-pushed
the
feat/add-recent-orgs-v2
branch
from
dca6c57 to
3aecef5
Compare
github-actions
bot
added
files-changed: 3
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
website/views/company.py (1)
136-138: Consider optimizing the query to fetch only required fields.The query fetches all fields from the Organization model, but the template only uses
name,created, andslug. Consider using.only()to reduce the data transferred from the database:- recent_organizations = Organization.objects.filter(is_active=True).order_by("-created")[:5] + recent_organizations = Organization.objects.filter(is_active=True).only("name", "slug", "created").order_by("-created")[:5]Additionally, consider caching this query since the registration page is public and may receive significant traffic, while the list of recent organizations doesn't change frequently.
website/templates/organization/register_organization.html (2)
220-249: Consider displaying organization logos and improving accessibility.The recently registered organizations section currently displays a generic building icon for all organizations. Consider using the actual organization logo when available to make the list more visually distinctive:
<div class="flex-shrink-0 flex items-center justify-center w-16 h-16 bg-gray-200 rounded-l-md"> {% if org.logo %} <img src="{{ org.logo.url }}" alt="{{ org.name }} logo" class="h-full w-full object-cover" /> {% else %} <svg class="h-8 w-8 text-gray-600" xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke-width="1.5" stroke="currentColor"> <path stroke-linecap="round" stroke-linejoin="round" d="M3.75 21h16.5M4.5 3h15M5.25 3v18m13.5-18v18M8.25 6h7.5m-7.5 3h7.5m-7.5 3h7.5m-7.5 3h7.5m-7.5 3h7.5" /> </svg> {% endif %} </div>Additionally, for accessibility, consider adding truncation for long organization names and making the link text more descriptive:
<a href="{{ org.get_absolute_url }}" class="font-medium text-gray-900 hover:text-gray-600 truncate" title="View {{ org.name }} details"> {{ org.name }} </a>
252-299: Consider mobile responsiveness and auto-dismiss for notifications.The notification container uses fixed positioning (
bottom-5 right-5), which could overlap with content or extend beyond the viewport on mobile devices. Consider adding responsive positioning:<div id="notification-container" class="fixed bottom-5 right-5 z-50 space-y-4 max-w-full sm:max-w-sm">Additionally, consider implementing auto-dismiss functionality for success messages to improve user experience:
// In your scripts block document.addEventListener('DOMContentLoaded', function() { // Auto-dismiss success messages after 5 seconds const successAlerts = document.querySelectorAll('[id^="alert-success-"]'); successAlerts.forEach(function(alert) { setTimeout(function() { alert.remove(); }, 5000); }); });Error messages should remain dismissable only via user action, as they may require the user's attention.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (3)
website/models.py(1 hunks)website/templates/organization/register_organization.html(1 hunks)website/views/company.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run Tests
- GitHub Check: docker-test
🔇 Additional comments (1)
website/models.py (1)
233-234: LGTM! Standard Django pattern for generating organization URLs.The
get_absolute_url()method correctly returns a reverse URL using the organization's slug, following Django conventions.
|
@DonnieBLT please review for merge |
|
Kindly address the nitpicks. Almost all of those seem useful to me. |
DonnieBLT
left a comment
DonnieBLT
left a comment
There was a problem hiding this comment.
please also add a screenshot
github-actions
bot
added
unresolved-conversations: 1
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
website/templates/organization/register_organization.html (1)
225-254: Add dark mode support to maintain consistency.The "Recently Registered Organizations" section is missing dark mode styling classes, while the rest of the template includes them. This creates a visual inconsistency when users switch to dark mode.
Apply these changes to add dark mode support:
- <h3 class="text-lg font-medium leading-6 text-gray-900">Recently Registered Organizations</h3> + <h3 class="text-lg font-medium leading-6 text-gray-900 dark:text-gray-100">Recently Registered Organizations</h3> - <p class="mt-1 text-sm text-gray-600">Check out the latest organizations that have joined our platform.</p> + <p class="mt-1 text-sm text-gray-600 dark:text-gray-400">Check out the latest organizations that have joined our platform.</p>- <div class="flex-shrink-0 flex items-center justify-center w-16 h-16 bg-gray-200 rounded-l-md"> - <svg class="h-8 w-8 text-gray-600" + <div class="flex-shrink-0 flex items-center justify-center w-16 h-16 bg-gray-200 dark:bg-gray-700 rounded-l-md"> + <svg class="h-8 w-8 text-gray-600 dark:text-gray-400"- <div class="flex flex-1 items-center justify-between truncate rounded-r-md border-b border-r border-t border-gray-200 bg-white"> + <div class="flex flex-1 items-center justify-between truncate rounded-r-md border-b border-r border-t border-gray-200 dark:border-gray-700 bg-white dark:bg-gray-800"> <div class="flex-1 truncate px-4 py-2 text-sm"> <a href="{{ org.get_absolute_url }}" - class="font-medium text-gray-900 hover:text-gray-600">{{ org.name }}</a> - <p class="text-gray-500">{{ org.created|timesince }} ago</p> + class="font-medium text-gray-900 dark:text-gray-100 hover:text-gray-600 dark:hover:text-gray-400">{{ org.name }}</a> + <p class="text-gray-500 dark:text-gray-400">{{ org.created|timesince }} ago</p>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (3)
website/models.py(1 hunks)website/templates/organization/register_organization.html(1 hunks)website/views/company.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- website/views/company.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run Tests
- GitHub Check: docker-test
🔇 Additional comments (1)
website/models.py (1)
242-243: LGTM! Standard Django pattern implemented correctly.The
get_absolute_url()method follows Django conventions and is consistent with similar methods inDomain(line 336) andHunt(line 439) models.
| <!-- Notification Messages --> | ||
| {% if messages %} | ||
| <div id="notification-container" | ||
| class="fixed bottom-5 right-5 z-50 space-y-4"> | ||
| {% for message in messages %} | ||
| {% if message.level_tag == 'error' %} | ||
| <div id="alert-error-{{ forloop.counter }}" | ||
| class="popup-errors flex items-start p-4 mb-4 w-80 text-red-800 border-l-4 border-[#e74c3c] bg-red-50 rounded-md shadow-lg" | ||
| role="alert"> | ||
| <div class="flex-shrink-0 mt-0.5"> | ||
| <i class="fas fa-exclamation-circle text-[#e74c3c]"></i> | ||
| </div> | ||
| {% else %} | ||
| <div id="alert-success-{{ forloop.counter }}" | ||
| class="popup-errors flex items-start p-4 mb-4 w-80 text-green-800 border-l-4 border-green-500 bg-green-50 rounded-md shadow-lg" | ||
| role="alert"> | ||
| <div class="flex-shrink-0 mt-0.5"> | ||
| <i class="fas fa-check-circle text-green-500"></i> | ||
| </div> | ||
| <div class="ml-3"> | ||
| <h3 class="text-sm font-medium text-green-800">{% trans "Success" %}</h3> | ||
| <div class="mt-1 text-sm text-green-700">{{ message }}</div> | ||
| </div> | ||
| <button type="button" | ||
| onclick="removeError('alert-success-{{ forloop.counter }}')" | ||
| class="ml-auto -mx-1.5 -my-1.5 bg-green-50 text-green-500 rounded-lg p-1.5 hover:bg-green-100 inline-flex h-8 w-8 items-center justify-center" | ||
| aria-label="{% trans 'Dismiss' %}"> | ||
| <span class="sr-only">{% trans "Dismiss" %}</span> | ||
| <i class="fas fa-times"></i> | ||
| </button> | ||
| <div class="ml-3"> | ||
| <h3 class="text-sm font-medium text-red-800">{% trans "Error" %}</h3> | ||
| <div class="mt-1 text-sm text-red-700">{{ message }}</div> | ||
| </div> | ||
| {% endif %} | ||
| {% endfor %} | ||
| </div> | ||
| {% endif %} | ||
| </div> | ||
| <button type="button" | ||
| onclick="removeError('alert-error-{{ forloop.counter }}')" | ||
| class="ml-auto -mx-1.5 -my-1.5 bg-red-50 text-red-500 rounded-lg p-1.5 hover:bg-red-100 inline-flex h-8 w-8 items-center justify-center" | ||
| aria-label="{% trans 'Dismiss' %}"> | ||
| <span class="sr-only">{% trans "Dismiss" %}</span> | ||
| <i class="fas fa-times"></i> | ||
| </button> | ||
| </div> | ||
| {% else %} | ||
| <div id="alert-success-{{ forloop.counter }}" | ||
| class="popup-errors flex items-start p-4 mb-4 w-80 text-green-800 border-l-4 border-green-500 bg-green-50 rounded-md shadow-lg" | ||
| role="alert"> | ||
| <div class="flex-shrink-0 mt-0.5"> | ||
| <i class="fas fa-check-circle text-green-500"></i> | ||
| </div> | ||
| <div class="ml-3"> | ||
| <h3 class="text-sm font-medium text-green-800">{% trans "Success" %}</h3> | ||
| <div class="mt-1 text-sm text-green-700">{{ message }}</div> | ||
| </div> | ||
| <button type="button" | ||
| onclick="removeError('alert-success-{{ forloop.counter }}')" | ||
| class="ml-auto -mx-1.5 -my-1.5 bg-green-50 text-green-500 rounded-lg p-1.5 hover:bg-green-100 inline-flex h-8 w-8 items-center justify-center" | ||
| aria-label="{% trans 'Dismiss' %}"> | ||
| <span class="sr-only">{% trans "Dismiss" %}</span> | ||
| <i class="fas fa-times"></i> | ||
| </button> | ||
| </div> | ||
| {% endif %} | ||
| {% endfor %} | ||
| </div> | ||
| {% endif %} |
There was a problem hiding this comment.
🧩 Analysis chain
Verify message level handling or add explicit checks.
The notification refactor assumes only two message types: error and everything else (treated as success). If the codebase can generate warning, info, or debug messages, they would incorrectly display with success (green) styling.
Run the following script to check if other message levels are used in the codebase:
If other message levels are found, consider adding explicit handling:
{% for message in messages %}
{% if message.level_tag == 'error' %}
<div id="alert-error-{{ forloop.counter }}"
class="popup-errors flex items-start p-4 mb-4 w-80 text-red-800 border-l-4 border-[#e74c3c] bg-red-50 rounded-md shadow-lg"
role="alert">
<!-- error content -->
</div>
- {% else %}
+ {% elif message.level_tag == 'success' %}
<div id="alert-success-{{ forloop.counter }}"
class="popup-errors flex items-start p-4 mb-4 w-80 text-green-800 border-l-4 border-green-500 bg-green-50 rounded-md shadow-lg"
role="alert">
<!-- success content -->
</div>
+ {% else %}
+ {# Handle warning, info, debug with appropriate styling #}
{% endif %}
{% endfor %}🏁 Script executed:
#!/bin/bash
# Search for Django messages calls with warning, info, or debug levels
rg -n --type=py -C2 'messages\.(warning|info|debug)\('
# Also check for level constants
rg -n --type=py -C2 'messages\.(WARNING|INFO|DEBUG)'Length of output: 7887
Add explicit handling for warning and info message levels.
Verification found that the codebase uses messages.warning() and messages.info() extensively throughout views (user.py, company.py, issue.py, hackathon.py, organization.py, staking_competitive.py, queue.py, core.py), but the template only conditionally renders error and implicitly treats all other levels as success. This causes warning and info messages to display with green success styling, creating semantic and UX confusion.
Update the template to explicitly handle each level:
{% for message in messages %}
{% if message.level_tag == 'error' %}
<div id="alert-error-{{ forloop.counter }}"
class="popup-errors flex items-start p-4 mb-4 w-80 text-red-800 border-l-4 border-[#e74c3c] bg-red-50 rounded-md shadow-lg"
role="alert">
<!-- error content -->
</div>
+ {% elif message.level_tag == 'warning' %}
+ <div id="alert-warning-{{ forloop.counter }}"
+ class="popup-errors flex items-start p-4 mb-4 w-80 text-amber-800 border-l-4 border-amber-500 bg-amber-50 rounded-md shadow-lg"
+ role="alert">
+ <div class="flex-shrink-0 mt-0.5">
+ <i class="fas fa-exclamation-triangle text-amber-500"></i>
+ </div>
+ <div class="ml-3">
+ <h3 class="text-sm font-medium text-amber-800">{% trans "Warning" %}</h3>
+ <div class="mt-1 text-sm text-amber-700">{{ message }}</div>
+ </div>
+ <button type="button"
+ onclick="removeError('alert-warning-{{ forloop.counter }}')"
+ class="ml-auto -mx-1.5 -my-1.5 bg-amber-50 text-amber-500 rounded-lg p-1.5 hover:bg-amber-100 inline-flex h-8 w-8 items-center justify-center"
+ aria-label="{% trans 'Dismiss' %}">
+ <span class="sr-only">{% trans "Dismiss" %}</span>
+ <i class="fas fa-times"></i>
+ </button>
+ </div>
+ {% elif message.level_tag == 'info' %}
+ <div id="alert-info-{{ forloop.counter }}"
+ class="popup-errors flex items-start p-4 mb-4 w-80 text-blue-800 border-l-4 border-blue-500 bg-blue-50 rounded-md shadow-lg"
+ role="alert">
+ <div class="flex-shrink-0 mt-0.5">
+ <i class="fas fa-info-circle text-blue-500"></i>
+ </div>
+ <div class="ml-3">
+ <h3 class="text-sm font-medium text-blue-800">{% trans "Info" %}</h3>
+ <div class="mt-1 text-sm text-blue-700">{{ message }}</div>
+ </div>
+ <button type="button"
+ onclick="removeError('alert-info-{{ forloop.counter }}')"
+ class="ml-auto -mx-1.5 -my-1.5 bg-blue-50 text-blue-500 rounded-lg p-1.5 hover:bg-blue-100 inline-flex h-8 w-8 items-center justify-center"
+ aria-label="{% trans 'Dismiss' %}">
+ <span class="sr-only">{% trans "Dismiss" %}</span>
+ <i class="fas fa-times"></i>
+ </button>
+ </div>
- {% else %}
+ {% elif message.level_tag == 'success' %}
<div id="alert-success-{{ forloop.counter }}"
class="popup-errors flex items-start p-4 mb-4 w-80 text-green-800 border-l-4 border-green-500 bg-green-50 rounded-md shadow-lg"
role="alert">
<!-- success content -->
</div>
+ {% else %}
+ {# Fallback for other message levels (e.g., debug) #}
+ <div id="alert-default-{{ forloop.counter }}"
+ class="popup-errors flex items-start p-4 mb-4 w-80 text-gray-800 border-l-4 border-gray-500 bg-gray-50 rounded-md shadow-lg"
+ role="alert">
+ <div class="flex-shrink-0 mt-0.5">
+ <i class="fas fa-info-circle text-gray-500"></i>
+ </div>
+ <div class="ml-3">
+ <h3 class="text-sm font-medium text-gray-800">{% trans "Message" %}</h3>
+ <div class="mt-1 text-sm text-gray-700">{{ message }}</div>
+ </div>
+ <button type="button"
+ onclick="removeError('alert-default-{{ forloop.counter }}')"
+ class="ml-auto -mx-1.5 -my-1.5 bg-gray-50 text-gray-500 rounded-lg p-1.5 hover:bg-gray-100 inline-flex h-8 w-8 items-center justify-center"
+ aria-label="{% trans 'Dismiss' %}">
+ <span class="sr-only">{% trans "Dismiss" %}</span>
+ <i class="fas fa-times"></i>
+ </button>
+ </div>
{% endif %}
{% endfor %}
github-actions
bot
added
unresolved-conversations: 2
Satish-Medar
force-pushed
the
feat/add-recent-orgs-v2
branch
from
daff229 to
05b8e81
Compare
github-actions
bot
added
files-changed: 7
|
Hi @Satish-Medar! This pull request has merge conflicts with the base branch that need to be resolved before it can be merged. To resolve the conflicts:
Once you push the resolved conflicts, this label and comment will be automatically updated. Thank you! 🙏 |
|
Hi @DonnieBLT and @sidd190 , thank you for the reviews! I have just: Pushed a new commit that fixes all the pre-commit formatting issues. Accepted/fixed all the coderabbitai bot's nitpicks. Added the local test screenshot to the PR description as requested. All checks should be green now and it should be ready to merge. Thanks! |
3 participants
Closes #4802
This is a new, clean PR for the "Recently Registered Organizations" feature, as requested by @DonnieBLT
The previous PR (#4801) was closed due to including unrelated files. This new PR contains only the 3 files necessary for the feature:
models.py: To add the missingget_absolute_urlfunction.views.py: To query the database for the orgs.register_organization.html: To display the new list.The feature has been tested locally and is working as expected.
Summary by CodeRabbit
Local Test Screenshot