Repository navigation
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e9350b237
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * @return {string} | ||
| */ | ||
| function normalizeEmptyByMonthDayValues(vcalendar) { | ||
| return vcalendar.replace(/(RRULE(?:;[^:\r\n]*)?:)([^\r\n]*)/gi, (line, prefix, rule) => { |
There was a problem hiding this comment.
When a CalDAV payload uses standard line folding before or within BYMONTHDAY (for example, RRULE:FREQ=MONTHLY;\r\n BYMONTHDAY=,24), this pattern stops at the physical CR/LF and never examines the continuation. ICAL.parse then unfolds the unchanged malformed value and rejects it, so the task/calendar disappearance this change is meant to prevent remains for folded recurrence rules; unfold the content line or include its continuations before sanitizing.
Useful? React with 👍 / 👎.
| * @return {string} | ||
| */ | ||
| function normalizeEmptyByMonthDayValues(vcalendar) { | ||
| return vcalendar.replace(/(RRULE(?:;[^:\r\n]*)?:)([^\r\n]*)/gi, (line, prefix, rule) => { |
There was a problem hiding this comment.
Whenever another property contains the substring RRULE:—for example the valid URI URL:https://example.test/inspect?RRULE:FREQ=MONTHLY;BYMONTHDAY=,24—this unanchored regexp treats the rest of that property as a recurrence rule and removes the comma from its value. The constructor stores the changed value in jCal, which updateTask later serializes via ICAL.stringify(task.jCal) in src/store/tasks.js:1114, silently corrupting URL or extension data on the next edit; restrict matching to an RRULE property name at the start of an unfolded content line.
Useful? React with 👍 / 👎.
Signed-off-by: Sean <sean0407@users.noreply.github.com>
5e9350b to
c156f6f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c156f6f52b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return vcalendar.replace(/(RRULE(?:;[^:\r\n]*)?:)([^\r\n]*)/gi, (line, prefix, rule) => { | ||
| return prefix + rule.replace(/(^|;)BYMONTHDAY=([^;]*)/i, (part, separator, days) => { | ||
| const normalizedDays = days.split(',').filter(Boolean).join(',') | ||
| return normalizedDays ? `${separator}BYMONTHDAY=${normalizedDays}` : '' |
There was a problem hiding this comment.
When BYMONTHDAY is the first rule part and every entry is empty, for example RRULE:BYMONTHDAY=,;FREQ=MONTHLY, separator is empty and this branch emits RRULE:;FREQ=MONTHLY. Recurrence parts may legally precede FREQ, but the resulting leading empty part is invalid and ical.js rejects it, so these tasks still fail to load instead of being normalized; when dropping the first part, consume the following semicolon as well.
Useful? React with 👍 / 👎.
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
Summary
BYMONTHDAYbefore parsing a task recurrence rule, so an invalid value such as,24does not make the task calendar disappear.Fixes #3265
Checks
npm test -- tests/javascript/unit/models/task.spec.js(80 passed)npx eslint src/models/task.js tests/javascript/unit/models/task.spec.js(passed)