Update linkedin - #94
Merged
Merged
Conversation
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
the
size:XL
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
Comment on lines
+209
to
217
| linkedin_credential = IntegrationCredential.objects.filter(provider="linkedin").first() | ||
| access_token = linkedin_credential.access_token if linkedin_credential else settings.LINKEDIN_ACCESS_TOKEN | ||
| refresh_token = linkedin_credential.refresh_token if linkedin_credential else settings.LINKEDIN_REFRESH_TOKEN | ||
|
|
||
| if not settings.LINKEDIN_ORGANIZATION_URN: | ||
| return "LinkedIn organization URN not configured in settings. Skipping post." | ||
|
|
||
| if not access_token and not (refresh_token and settings.LINKEDIN_CLIENT_ID and settings.LINKEDIN_CLIENT_SECRET): | ||
| return "LinkedIn API credentials not configured in settings. Skipping post." |
There was a problem hiding this comment.
🟠 Architect Review — HIGH
The token-selection logic in post_event_to_linkedin assumes that if an IntegrationCredential row exists it is authoritative, even when its access_token/refresh_token fields are empty. If a linkedin IntegrationCredential has null tokens while LINKEDIN_ACCESS_TOKEN / LINKEDIN_REFRESH_TOKEN are set in settings (e.g., after running linkedin_oauth --show-url, which creates the row but does not yet store tokens), the code selects None for both tokens and skips posting, regressing from the previous behavior that used the settings tokens.
Suggestion: When selecting tokens, fall back per field to settings (use the credential's access/refresh token when present, otherwise the corresponding settings value), and treat a credential with no usable tokens as equivalent to "no credential" so existing LinkedIn posting continues to work until OAuth token storage completes.
Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is an **Architect / Logical Review** comment left during a code review. These reviews are first-class, important findings — not optional suggestions. Do NOT dismiss this as a 'big architectural change' just because the title says architect review; most of these can be resolved with a small, localized fix once the intent is understood.
**Path:** src/django_project/web/tasks.py
**Line:** 209:217
**Comment:**
*HIGH: The token-selection logic in post_event_to_linkedin assumes that if an IntegrationCredential row exists it is authoritative, even when its access_token/refresh_token fields are empty. If a linkedin IntegrationCredential has null tokens while LINKEDIN_ACCESS_TOKEN / LINKEDIN_REFRESH_TOKEN are set in settings (e.g., after running linkedin_oauth --show-url, which creates the row but does not yet store tokens), the code selects None for both tokens and skips posting, regressing from the previous behavior that used the settings tokens.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
If a suggested approach is provided above, use it as the authoritative instruction. If no explicit code suggestion is given, you MUST still draft and apply your own minimal, localized fix — do not punt back with 'no suggestion provided, review manually'. Keep the change as small as possible: add a guard clause, gate on a loading state, reorder an await, wrap in a conditional, etc. Do not refactor surrounding code or expand scope beyond the finding.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+36
to
+37
| if not client_id or not client_secret: | ||
| raise CommandError("LINKEDIN_CLIENT_ID and LINKEDIN_CLIENT_SECRET must be configured.") |
There was a problem hiding this comment.
Suggestion: This validation blocks the "show authorization URL" flow unless both client ID and client secret are configured, but generating the authorization URL only requires the client ID. As written, the command can fail before showing the URL even though exchange is not being performed. [incorrect condition logic]
Severity Level: Major ⚠️
- ⚠️ Admins cannot generate LinkedIn OAuth URL without secret.
- ⚠️ Initial LinkedIn setup via management command is more brittle.Steps of Reproduction ✅
1. Configure only `LINKEDIN_CLIENT_ID` in the env file loaded by `ENV_PATH` (see
`core/settings.py:23-31`), leaving `LINKEDIN_CLIENT_SECRET` unset or empty so
`settings.LINKEDIN_CLIENT_SECRET` is falsy.
2. Run the management command `python manage.py linkedin_oauth --redirect-uri
https://example.com/linkedin/callback --show-url` which invokes `Command.handle()` in
`src/django_project/web/management/commands/linkedin_oauth.py:31-77`.
3. In `handle()`, `client_id = settings.LINKEDIN_CLIENT_ID` and `client_secret =
settings.LINKEDIN_CLIENT_SECRET` are read (lines 32-33), and the validation at lines 36-37
(`if not client_id or not client_secret: raise CommandError(...)`) triggers because
`client_secret` is missing.
4. As a result, the command raises `CommandError` before reaching the authorization URL
generation at lines 58-60 (`client.build_authorization_url(...)`), even though generating
the URL only requires a client ID and not the secret.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/django_project/web/management/commands/linkedin_oauth.py
**Line:** 36:37
**Comment:**
*Incorrect Condition Logic: This validation blocks the "show authorization URL" flow unless both client ID and client secret are configured, but generating the authorization URL only requires the client ID. As written, the command can fail before showing the URL even though exchange is not being performed.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+39
to
+40
| credential_model = apps.get_model("web", "IntegrationCredential") | ||
| credential, _ = credential_model.objects.get_or_create(provider="linkedin") |
There was a problem hiding this comment.
Suggestion: The command creates a database credential row before knowing whether token exchange will happen. Running it only to print the URL leaves an empty credential record, which can later interfere with token resolution logic. Delay get_or_create until you actually exchange and persist tokens. [logic error]
Severity Level: Critical 🚨
- ❌ Running URL-only OAuth command creates unusable LinkedIn credential.
- ❌ Empty credential row later blocks automated LinkedIn event posting.Steps of Reproduction ✅
1. Start with no `IntegrationCredential` row for provider `"linkedin"` in the database
(model defined in `src/django_project/web/models.py:84-92`).
2. Run `python manage.py linkedin_oauth --redirect-uri
https://example.com/linkedin/callback` without a `--code` argument so the command is used
only to show the authorization URL (entrypoint `Command.handle()` at
`linkedin_oauth.py:31`).
3. In `handle()`, before any decision about `code`, lines 39-40 execute
`credential_model.objects.get_or_create(provider="linkedin")`, creating a new
`IntegrationCredential` with `access_token` and `refresh_token` left `NULL`/empty because
those fields default blank/nullable (see `models.py:88-89`).
4. Later, when `web.tasks.post_event_to_linkedin()` runs (task defined at
`src/django_project/web/tasks.py:203-236` and invoked by
`launch_reminders_for_tomorrows_events()` at `tasks.py:260-288`), it loads
`linkedin_credential = IntegrationCredential.objects.filter(provider="linkedin").first()`
(line 209). Because the credential row exists but has empty tokens, downstream logic at
lines 210-217 treats `access_token` and `refresh_token` as missing and may skip posting to
LinkedIn, even though environment-based settings tokens could be valid. This undesired
side effect comes solely from having run the command once in URL-only mode.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/django_project/web/management/commands/linkedin_oauth.py
**Line:** 39:40
**Comment:**
*Logic Error: The command creates a database credential row before knowing whether token exchange will happen. Running it only to print the URL leaves an empty credential record, which can later interfere with token resolution logic. Delay `get_or_create` until you actually exchange and persist tokens.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+209
to
+211
| linkedin_credential = IntegrationCredential.objects.filter(provider="linkedin").first() | ||
| access_token = linkedin_credential.access_token if linkedin_credential else settings.LINKEDIN_ACCESS_TOKEN | ||
| refresh_token = linkedin_credential.refresh_token if linkedin_credential else settings.LINKEDIN_REFRESH_TOKEN |
There was a problem hiding this comment.
Suggestion: The credential lookup no longer falls back to settings when a LinkedIn credential row exists but has empty token fields. If an empty IntegrationCredential record is present, both tokens become None even when valid settings tokens exist, and posting will be skipped as "not configured." Use per-field fallback (db_value or settings_value) so legacy settings tokens still work. [incorrect variable usage]
Severity Level: Critical 🚨
- ❌ LinkedIn posts skipped whenever a blank credential row exists.
- ❌ Scheduled LinkedIn reminders never send despite valid settings tokens.Steps of Reproduction ✅
1. Ensure valid LinkedIn tokens are configured via environment variables so
`settings.LINKEDIN_ACCESS_TOKEN` and `settings.LINKEDIN_REFRESH_TOKEN` are non-empty (env
loaded in `core/settings.py:23-31`).
2. Create an `IntegrationCredential` row with `provider="linkedin"` but leave
`access_token` and `refresh_token` blank/NULL (e.g., via Django admin or shell; model
fields at `web/models.py:84-92` allow blank/null).
3. Trigger any code path that posts events to LinkedIn, such as
`launch_reminders_for_tomorrows_events()` at `web/tasks.py:260-290`, which enqueues
`post_event_to_linkedin.s(event.pk, is_new=False)` on line 287, or call
`post_event_to_linkedin(event_pk, is_new=True)` directly.
4. Inside `post_event_to_linkedin()` (`tasks.py:204-236`), line 209 loads the empty
credential, line 210 sets `access_token = linkedin_credential.access_token` (which is
`None`), and line 211 sets `refresh_token = linkedin_credential.refresh_token` (also
`None`); the code never falls back to `settings.LINKEDIN_ACCESS_TOKEN` or
`settings.LINKEDIN_REFRESH_TOKEN` because the presence of a credential short-circuits the
fallback.
5. The configuration check at line 216 (`if not access_token and not (refresh_token and
settings.LINKEDIN_CLIENT_ID and settings.LINKEDIN_CLIENT_SECRET): return "LinkedIn API
credentials not configured in settings. Skipping post."`) evaluates true, so the task
returns early and no LinkedIn post is made, despite valid tokens being present in
settings.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/django_project/web/tasks.py
**Line:** 209:211
**Comment:**
*Incorrect Variable Usage: The credential lookup no longer falls back to settings when a LinkedIn credential row exists but has empty token fields. If an empty `IntegrationCredential` record is present, both tokens become `None` even when valid settings tokens exist, and posting will be skipped as "not configured." Use per-field fallback (`db_value or settings_value`) so legacy settings tokens still work.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+191
to
+206
| lines = self.env_path.read_text(encoding="utf-8").splitlines() | ||
| replacements = { | ||
| "LINKEDIN_ACCESS_TOKEN": self.access_token, | ||
| "LINKEDIN_REFRESH_TOKEN": self.refresh_token, | ||
| } | ||
|
|
||
| for key, value in replacements.items(): | ||
| serialized_value = json.dumps(value) if value is not None else '""' | ||
| for index, line in enumerate(lines): | ||
| if line.startswith(f"{key}="): | ||
| lines[index] = f"{key}={serialized_value}" | ||
| break | ||
| else: | ||
| lines.append(f"{key}={serialized_value}") | ||
|
|
||
| self.env_path.write_text("\n".join(lines) + "\n", encoding="utf-8") |
There was a problem hiding this comment.
Suggestion: Updating the env file uses an unlocked read-modify-write sequence, so concurrent refreshes can overwrite each other or produce partial/truncated writes. This is a race condition in multi-worker environments; use a file lock plus atomic replace (write temp file then rename) to persist tokens safely. [race condition]
Severity Level: Major ⚠️
- ❌ Concurrent env-based token refresh can corrupt LinkedIn credentials.
- ⚠️ Future multi-worker deployments using env persistence become unstable.Steps of Reproduction ✅
1. Use the existing test helper `LinkedInOrganizationClientTests.build_client()` in
`src/django_project/tests/unit/web/test_linkedin_notifier.py:40-48` (or equivalent
production code) to construct a `LinkedInOrganizationClient` with `env_path` pointing to a
shared env file and `credential=None`, so `_persist_tokens()` takes the env-file branch
(see `linkedin.py:157-183`).
2. In two separate worker processes or threads (e.g., two Celery workers both refreshing
tokens), call `client.refresh_access_token()` concurrently; this method ultimately calls
`_persist_tokens()` at `linkedin.py:157-182` and then executes the env-file write logic at
lines 191-206 when no DB credential is used.
3. Each concurrent `_persist_tokens()` call executes `lines =
self.env_path.read_text(...).splitlines()` (line 191), mutates the in-memory `lines` list
for `"LINKEDIN_ACCESS_TOKEN"` and `"LINKEDIN_REFRESH_TOKEN"` entries (lines 197-204), then
writes the entire file back with `self.env_path.write_text("\n".join(lines) + "\n", ...)`
(line 206) without any file lock or atomic rename.
4. Because these read-modify-write cycles are unsynchronized, one process can read an
outdated version of the env file and overwrite the other process's newer contents, or
interleaved writes can truncate or partially write the file, leading to corrupted or lost
LinkedIn token values in the env file used by `core/settings.py:23-31`.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/django_project/web/utilities/notifiers/linkedin.py
**Line:** 191:206
**Comment:**
*Race Condition: Updating the env file uses an unlocked read-modify-write sequence, so concurrent refreshes can overwrite each other or produce partial/truncated writes. This is a race condition in multi-worker environments; use a file lock plus atomic replace (write temp file then rename) to persist tokens safely.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
|
CodeAnt AI finished reviewing your PR. |
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
size:XL
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
1 participant
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.
User description
Pull Request
Description:
Updating LinkedIn notifier to automatically refresh a token for posts to the SpokaneTech LinkedIn page.
Checklist:
Testing:
CodeAnt-AI Description
Add LinkedIn OAuth setup, token refresh, and retry support
What Changed
Impact
✅ Fewer LinkedIn post failures✅ Faster recovery after token expiry✅ Easier LinkedIn token setup💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.