-
-
Notifications
You must be signed in to change notification settings - Fork 284
Feat: Add recently registered orgs to registration page #4801
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
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds a "Recently Registered Organizations" section to the organization registration page, passes the five most-recent Organization records from the GET view to the template, adds Organization.get_absolute_url, and applies multiple small formatting, YAML, and CSS fixes across the repo. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant View as RegisterOrganizationView
participant DB as Database
participant Template
User->>View: GET /organization/register
View->>DB: Query Organization.objects.order_by("-created")[:5]
DB-->>View: recent_organizations (0..5)
View->>Template: render(template, context incl. recent_organizations)
Template-->>User: HTML (form + Recently Registered Organizations section)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Knowledge base: Disabled due to 📒 Files selected for processing (2)
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 |
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
website/templates/organization/register_organization.html (1)
232-237: Remove extra whitespace for consistency.Line 233 contains unnecessary whitespace that can be removed for cleaner formatting.
Apply this diff:
<li class="col-span-1 flex rounded-md shadow-sm"> <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>
📜 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 (2)
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/templates/organization/register_organization.html (1)
220-251: LGTM! Well-structured section with good responsive design.The "Recently Registered Organizations" section is properly implemented with:
- Conditional rendering to avoid empty state issues
- Responsive grid layout (1/2/3 columns based on screen size)
- Good accessibility with
role="list"attribute- Clean Tailwind CSS styling
c8b3f75 to
c8dbc1d
Compare
c8dbc1d to
a1308be
Compare
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
website/documents/BltWeeklyActivityOfContributor.md (1)
44-44: Simplify verbose phrasing in the Scalability point.Line 44 uses the wordy phrase "a large number of," which can be replaced with a clearer alternative like "many," "numerous," or a specific quantifier. This will improve readability and conciseness of the documentation.
Apply this diff to improve clarity:
- **Scalability**: The table is designed to handle a large number of users and activity metrics, maintaining performance and user experience even with extensive data. + **Scalability**: The table is designed to handle many users and activity metrics, maintaining performance and user experience even with extensive data.Alternatively, if a specific scale is known, consider using an exact number (e.g., "thousands of users").
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
⛔ Files ignored due to path filters (8)
website/static/images/tech/ml.svgis excluded by!**/*.svgwebsite/static/images/tech/nlp.svgis excluded by!**/*.svgwebsite/static/images/tech/oauth.svgis excluded by!**/*.svgwebsite/static/images/tech/python.svgis excluded by!**/*.svgwebsite/static/images/tech/sql.svgis excluded by!**/*.svgwebsite/static/images/tech/tailwind.svgis excluded by!**/*.svgwebsite/static/img/tomato-svgrepo-com.svgis excluded by!**/*.svgwebsite/static/js/jquery.sparkline.min.jsis excluded by!**/*.min.js
📒 Files selected for processing (40)
.github/codeql-config.yml(1 hunks).github/workflows/add-files-changed-label.yml(1 hunks).lgtm.yml(1 hunks)BACON/bacon-etch.yaml(1 hunks)BACON/ord-server/example-split.yaml(1 hunks)BACON/regtest/bitcoin.conf(1 hunks)Dockerfile(1 hunks)Procfile(1 hunks)docker-compose.yml(1 hunks)docs/features.md(1 hunks)issues/ValueError_staking_pool.md(1 hunks)website/documents/BltAboutUs.md(1 hunks)website/documents/BltBLTV.md(1 hunks)website/documents/BltChangePassword.md(1 hunks)website/documents/BltCommunityMembers.md(1 hunks)website/documents/BltCompanyDashboard.md(1 hunks)website/documents/BltCompanyListingPage.md(1 hunks)website/documents/BltCompanyScoreboard.md(1 hunks)website/documents/BltDetails.md(1 hunks)website/documents/BltDetailsFromOwasp.md(1 hunks)website/documents/BltGlobalLeaderboard.md(1 hunks)website/documents/BltInvite.md(1 hunks)website/documents/BltLoginPage.md(1 hunks)website/documents/BltSignUpPage.md(1 hunks)website/documents/BltStartaBughunt.md(1 hunks)website/documents/BltStats.md(1 hunks)website/documents/BltTerms.md(1 hunks)website/documents/BltTrademarksSearch.md(1 hunks)website/documents/BltTrademarksSearchResults.md(1 hunks)website/documents/BltUserProfile.md(1 hunks)website/documents/BltWeeklyActivityOfContributor.md(1 hunks)website/models.py(1 hunks)website/static/css/app.min.css(1 hunks)website/static/css/checkInModal.css(1 hunks)website/static/js/issue.js(1 hunks)website/static/js/messages.js(1 hunks)website/static/js/reminder_settings.js(1 hunks)website/static/organization/js/hunt_controller.js(1 hunks)website/templates/organization/register_organization.html(1 hunks)website/views/company.py(1 hunks)
✅ Files skipped from review due to trivial changes (13)
- website/documents/BltBLTV.md
- website/static/css/app.min.css
- BACON/regtest/bitcoin.conf
- website/documents/BltTerms.md
- website/static/organization/js/hunt_controller.js
- website/documents/BltStartaBughunt.md
- website/documents/BltGlobalLeaderboard.md
- BACON/ord-server/example-split.yaml
- website/static/js/reminder_settings.js
- website/documents/BltDetails.md
- website/documents/BltAboutUs.md
- website/static/css/checkInModal.css
- .lgtm.yml
🚧 Files skipped from review as they are similar to previous changes (22)
- issues/ValueError_staking_pool.md
- website/documents/BltDetailsFromOwasp.md
- website/static/js/issue.js
- website/documents/BltCompanyScoreboard.md
- website/models.py
- website/documents/BltChangePassword.md
- website/documents/BltSignUpPage.md
- .github/workflows/add-files-changed-label.yml
- website/documents/BltStats.md
- website/documents/BltTrademarksSearch.md
- website/views/company.py
- website/documents/BltCompanyListingPage.md
- website/static/js/messages.js
- website/documents/BltCompanyDashboard.md
- website/documents/BltUserProfile.md
- Procfile
- website/templates/organization/register_organization.html
- website/documents/BltCommunityMembers.md
- website/documents/BltLoginPage.md
- .github/codeql-config.yml
- Dockerfile
- docs/features.md
🧰 Additional context used
🪛 LanguageTool
website/documents/BltWeeklyActivityOfContributor.md
[style] ~44-~44: To reduce wordiness, try specifying a number or using “many” or “numerous” instead.
Context: ...lity**: The table is designed to handle a large number of users and activity metrics, maintaining...
(LARGE_NUMBER_OF)
⏰ 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 (5)
website/documents/BltTrademarksSearchResults.md (1)
42-42: Good: Minor formatting consistency improvement.Converting the non-standard
--bullet to the standard markdown-aligns with the formatting used throughout the rest of the document.docker-compose.yml (2)
3-3: Verify PostgreSQL version aligns with PR objectives.The PR objectives state that
docker-compose.ymlshould be updated to pinpostgres:16to resolve local setup breakage. However, Line 3 specifiespostgres:17.6, which is a newer version. Please clarify whether this is intentional or if the version should bepostgres:16as described in the PR objectives.
54-55: YAML structure for named volumes is correct.The volumes section correctly uses mapping syntax (
postgres_db:) rather than list syntax. This is the proper structure for defining named volumes in docker-compose.website/documents/BltInvite.md (1)
37-37: This documentation change is incidental to the PR objectives.This file is being updated only to fix EOF formatting (adding proper newline at end of file), which is unrelated to the PR objective of adding recently registered organizations to the registration page. The documentation content itself is well-structured and comprehensive.
BACON/bacon-etch.yaml (1)
12-12: Clarify scope and verify the structural change.This file appears unrelated to the PR objective (adding recently registered organizations to the registration page). Additionally, there's an inconsistency: the AI summary claims a structural change from a YAML list item to a plain mapping, but the current code shows the list item format with the
-prefix.Could you confirm:
- Whether this file should be included in this PR?
- What the original state of line 12 was before this change?
a1308be to
a4435fc
Compare
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
website/views/company.py (1)
136-138: Consider query optimization and filtering.The query fetches all Organization fields but the template may only need a subset (e.g., name, slug, logo, created). Consider using
.only()or.values()to reduce data transfer. Additionally, verify whether inactive organizations (is_active=False) should be included in the recently registered list shown to public visitors.Apply this optimization if the template only uses specific fields:
-recent_organizations = Organization.objects.all().order_by("-created")[:5] +recent_organizations = ( + Organization.objects + .filter(is_active=True) + .only("name", "slug", "logo", "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 ignored due to path filters (8)
website/static/images/tech/ml.svgis excluded by!**/*.svgwebsite/static/images/tech/nlp.svgis excluded by!**/*.svgwebsite/static/images/tech/oauth.svgis excluded by!**/*.svgwebsite/static/images/tech/python.svgis excluded by!**/*.svgwebsite/static/images/tech/sql.svgis excluded by!**/*.svgwebsite/static/images/tech/tailwind.svgis excluded by!**/*.svgwebsite/static/img/tomato-svgrepo-com.svgis excluded by!**/*.svgwebsite/static/js/jquery.sparkline.min.jsis excluded by!**/*.min.js
📒 Files selected for processing (40)
.github/codeql-config.yml(1 hunks).github/workflows/add-files-changed-label.yml(1 hunks).lgtm.yml(1 hunks)BACON/bacon-etch.yaml(1 hunks)BACON/ord-server/example-split.yaml(1 hunks)BACON/regtest/bitcoin.conf(1 hunks)Dockerfile(1 hunks)Procfile(1 hunks)docker-compose.yml(1 hunks)docs/features.md(1 hunks)issues/ValueError_staking_pool.md(1 hunks)website/documents/BltAboutUs.md(1 hunks)website/documents/BltBLTV.md(1 hunks)website/documents/BltChangePassword.md(1 hunks)website/documents/BltCommunityMembers.md(1 hunks)website/documents/BltCompanyDashboard.md(1 hunks)website/documents/BltCompanyListingPage.md(1 hunks)website/documents/BltCompanyScoreboard.md(1 hunks)website/documents/BltDetails.md(1 hunks)website/documents/BltDetailsFromOwasp.md(1 hunks)website/documents/BltGlobalLeaderboard.md(1 hunks)website/documents/BltInvite.md(1 hunks)website/documents/BltLoginPage.md(1 hunks)website/documents/BltSignUpPage.md(1 hunks)website/documents/BltStartaBughunt.md(1 hunks)website/documents/BltStats.md(1 hunks)website/documents/BltTerms.md(1 hunks)website/documents/BltTrademarksSearch.md(1 hunks)website/documents/BltTrademarksSearchResults.md(1 hunks)website/documents/BltUserProfile.md(1 hunks)website/documents/BltWeeklyActivityOfContributor.md(1 hunks)website/models.py(1 hunks)website/static/css/app.min.css(1 hunks)website/static/css/checkInModal.css(1 hunks)website/static/js/issue.js(1 hunks)website/static/js/messages.js(1 hunks)website/static/js/reminder_settings.js(1 hunks)website/static/organization/js/hunt_controller.js(1 hunks)website/templates/organization/register_organization.html(1 hunks)website/views/company.py(1 hunks)
✅ Files skipped from review due to trivial changes (12)
- website/documents/BltSignUpPage.md
- website/static/organization/js/hunt_controller.js
- website/documents/BltInvite.md
- website/static/js/reminder_settings.js
- website/static/js/issue.js
- BACON/ord-server/example-split.yaml
- .github/workflows/add-files-changed-label.yml
- website/documents/BltCommunityMembers.md
- issues/ValueError_staking_pool.md
- website/documents/BltGlobalLeaderboard.md
- .lgtm.yml
- website/static/js/messages.js
🚧 Files skipped from review as they are similar to previous changes (18)
- website/documents/BltUserProfile.md
- website/documents/BltBLTV.md
- website/documents/BltDetailsFromOwasp.md
- website/documents/BltDetails.md
- website/documents/BltCompanyListingPage.md
- website/documents/BltCompanyDashboard.md
- Dockerfile
- website/static/css/app.min.css
- website/documents/BltStats.md
- website/static/css/checkInModal.css
- website/templates/organization/register_organization.html
- website/documents/BltAboutUs.md
- website/documents/BltTrademarksSearchResults.md
- website/documents/BltTrademarksSearch.md
- website/documents/BltChangePassword.md
- website/documents/BltStartaBughunt.md
- BACON/regtest/bitcoin.conf
- BACON/bacon-etch.yaml
🧰 Additional context used
🪛 LanguageTool
website/documents/BltWeeklyActivityOfContributor.md
[style] ~44-~44: To reduce wordiness, try specifying a number or using “many” or “numerous” instead.
Context: ...lity**: The table is designed to handle a large number of users and activity metrics, maintaining...
(LARGE_NUMBER_OF)
🪛 markdownlint-cli2 (0.18.1)
website/documents/BltTerms.md
94-94: Bare URL used
(MD034, no-bare-urls)
⏰ 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 (11)
docs/features.md (1)
180-180: No review needed for whitespace formatting change.This is a cosmetic trailing whitespace removal with no impact on documentation content or functionality.
website/documents/BltWeeklyActivityOfContributor.md (1)
44-44: LGTM!Formatting correction to use consistent single-dash bullet prefix.
website/documents/BltTerms.md (1)
94-94: LGTM!Added trailing newline follows standard practice.
.github/codeql-config.yml (1)
43-43: LGTM!Added trailing newline follows YAML best practices.
website/documents/BltCompanyScoreboard.md (1)
45-45: LGTM!Formatting correction to use consistent single-dash bullet prefix.
Procfile (1)
3-3: Clarify the Procfile configuration.Line 3 remains commented out (starts with
#), so Gunicorn is not actually enabled. If the intent is to switch from uvicorn (line 2) to Gunicorn, remove the#prefix. If both are kept for reference, consider adding a comment explaining which server is active.docker-compose.yml (2)
55-55: LGTM!Correct YAML syntax fix—named volumes should be declared as a mapping, not a list.
3-3: Note: PostgreSQL version discrepancy.The PR summary mentions pinning
postgres:16to resolve setup issues, but line 3 usespostgres:17.6. Verify this is intentional or if the summary needs updating.website/models.py (1)
234-236: LGTM!The
get_absolute_url()method correctly uses Django'sreverse()to generate the organization detail URL from its slug. This enables template usage of{{ org.get_absolute_url }}as required by the new "Recently Registered Organizations" section.website/documents/BltLoginPage.md (2)
47-47: Minor formatting consistency improvement.The formatting change normalizes the bullet point prefix for "Form Persistence" to align with Markdown conventions. This is a low-impact documentation update with no functional consequences.
1-47: Documentation file appears unrelated to PR objective.This file contains only formatting adjustments and is unrelated to the primary PR objective of adding a "Recently Registered Organizations" section. While the changes are harmless, the scope seems misaligned.
Note: The core features mentioned in the AI summary (backend view changes in RegisterOrganizationView, Organization.get_absolute_url method addition, template updates, docker-compose modifications) are not provided in this review. Please ensure those files are included for a comprehensive review.
DonnieBLT
left a comment
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.
this has too many changed files it should be one or two files changed
|
too many files changed |
Closes #4802
This PR adds the "Recently Registered Organizations" list to the organization registration page, as suggested by @DonnieBLT in the comments of PR #4705.
That original PR (#4705) had merge conflicts and failing checks. This is a new, clean implementation of that feature on an up-to-date branch.
This PR does:
This should be ready to test and merge.
Summary by CodeRabbit
New Features
Deployment
Documentation & Configuration
Style