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

fix: Allow to refresh feeds if user isn't verified #2694

Merged
merged 1 commit into from
Dec 3, 2019

Conversation

marienfressinaud
Copy link
Member

While I was looking at the number of articles of my users, I discovered
some of them had none, while having a bunch of feeds though. I took a
look at the logs generated by app/actualize_script.php and discovered
that the script stopped strangely (in this example, "OK" for denise is
expected, and more users too):

FreshRSS[1681]: FreshRSS Start feeds actualization...
Starting feed actualization at 2019-11-29T16:37:19+00:00
Actualize alice...
Actualize denise...
Results:
alice OK
denise

After digging a bit, I quickly realized the script stopped always on
users who didn't validate their emails. And indeed, we trigger a
Minz_Request::forward(..., true) for these users, in the FreshRSS
class. This function calls the exit function, which stops the script.

This patch only allows the feed#actualize action to be executed for
unverified users in order to avoid an early-exit. This is a quick-win
solution, but I don't think it's a good one on the long term. I'll
propose an alternative in another patch, later.

While I was looking at the number of articles of my users, I discovered
some of them had none, while having a bunch of feeds though. I took a
look at the logs generated by `app/actualize_script.php` and discovered
that the script stopped strangely (in this example, "OK" for denise is
expected, and more users too):

```
FreshRSS[1681]: FreshRSS Start feeds actualization...
Starting feed actualization at 2019-11-29T16:37:19+00:00
Actualize alice...
Actualize denise...
Results:
alice OK
denise
```

After digging a bit, I quickly realized the script stopped always on
users who didn't validate their emails. And indeed, we trigger a
`Minz_Request::forward(..., true)` for these users, in the `FreshRSS`
class. This function calls the `exit` function, which stops the script.

This patch only allows the feed#actualize action to be executed for
unverified users in order to avoid an early-`exit`. This is a quick-win
solution, but I don't think it's a good one on the long term. I'll
propose an alternative in another patch, later.
marienfressinaud added a commit to flusio/xExtension-Flus that referenced this pull request Nov 29, 2019
@Alkarex Alkarex added this to the 1.16.0 milestone Dec 2, 2019
@Alkarex
Copy link
Member

Alkarex commented Dec 2, 2019

Looks fine for now

@Alkarex Alkarex merged commit 68c006b into FreshRSS:dev Dec 3, 2019
@marienfressinaud marienfressinaud deleted the fix/actualize-if-user-not-validated branch December 17, 2019 15:46
javerous pushed a commit to javerous/FreshRSS that referenced this pull request Jan 20, 2020
While I was looking at the number of articles of my users, I discovered
some of them had none, while having a bunch of feeds though. I took a
look at the logs generated by `app/actualize_script.php` and discovered
that the script stopped strangely (in this example, "OK" for denise is
expected, and more users too):

```
FreshRSS[1681]: FreshRSS Start feeds actualization...
Starting feed actualization at 2019-11-29T16:37:19+00:00
Actualize alice...
Actualize denise...
Results:
alice OK
denise
```

After digging a bit, I quickly realized the script stopped always on
users who didn't validate their emails. And indeed, we trigger a
`Minz_Request::forward(..., true)` for these users, in the `FreshRSS`
class. This function calls the `exit` function, which stops the script.

This patch only allows the feed#actualize action to be executed for
unverified users in order to avoid an early-`exit`. This is a quick-win
solution, but I don't think it's a good one on the long term. I'll
propose an alternative in another patch, later.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants