D5729: #379003: Fix National Geographic POTD provider

[David Edmundson]

This revision was automatically updated to reflect the committed changes.
Closed by commit R114:4ed05974c6a1: Replace XML parsing in National Geographic 
dataengine (authored by vitali, committed by davidedmundson).

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5729?vs=14202=16484

REVISION DETAIL
  https://phabricator.kde.org/D5729

AFFECTED FILES
  dataengines/potd/natgeoprovider.cpp

To: vitali, sebas, mart
Cc: mart, Zren, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, lukas

D5729: #379003: Fix National Geographic POTD provider

[Marco Martin]

mart accepted this revision.
mart added a comment.


  go for it

REPOSITORY
  R114 Plasma Addons

REVISION DETAIL
  https://phabricator.kde.org/D5729

To: vitali, sebas, mart
Cc: mart, Zren, plasma-devel, ZrenBot, spstarr, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, lukas

D5729: #379003: Fix National Geographic POTD provider

[Juri Vitali]

vitali added a comment.


  So, what do we want to do with this patch?
  I've been using it for a week now, and didn't notice any problems. Has anyone 
else been testing it?
  
  As for the link, I am for keeping the http version, as it is the one used by 
NatGeo, so it would be less likely to be broken in the near future. If and when 
they will move to TLS it should be their responsibility to issue the right 
redirect.
  Anyway, any decision will be fine for me, as long as it works.
  
  Lastly, any opinion on the regex?

REPOSITORY
  R114 Plasma Addons

REVISION DETAIL
  https://phabricator.kde.org/D5729

To: vitali, sebas
Cc: Zren, plasma-devel, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, lukas

D5729: #379003: Fix National Geographic POTD provider

[Chris Holland]

Zren added inline comments.

INLINE COMMENTS

> sebas wrote in natgeoprovider.cpp:96
> Why this change? Moving away from https to http seems backward...

The https redirects to http (HTTP 302 temp move), then redirects to add the 
trailing slash (HTTP 301 Perm Move).

F3744087: 2017-05-06___19-22-22.png 

REPOSITORY
  R114 Plasma Addons

REVISION DETAIL
  https://phabricator.kde.org/D5729

To: vitali, sebas
Cc: Zren, plasma-devel, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, lukas

D5729: #379003: Fix National Geographic POTD provider

[Juri Vitali]

vitali planned changes to this revision.
vitali added inline comments.

INLINE COMMENTS

> natgeoprovider.cpp:61
> +
> +
> re.setPattern("^$");
> +

An alternative regex may be 
`"^$"`, so to match any 
possible future fields before `property`, and between `property` and `content`.

> sebas wrote in natgeoprovider.cpp:96
> Why this change? Moving away from https to http seems backward...

The versions with https or without the trailing slash were not working for me, 
so I changed it to that one, but it could as well be just be my mistake.

REPOSITORY
  R114 Plasma Addons

REVISION DETAIL
  https://phabricator.kde.org/D5729

To: vitali, sebas
Cc: plasma-devel, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, lukas

D5729: #379003: Fix National Geographic POTD provider

[Sebastian Kügler]

sebas added inline comments.

INLINE COMMENTS

> natgeoprovider.cpp:96
>  {
> -const QUrl url( QLatin1String( 
> "https://www.nationalgeographic.com/photography/photo-of-the-day; ) );
> +const QUrl url( QLatin1String( 
> "http://www.nationalgeographic.com/photography/photo-of-the-day/; ) );
>  KIO::StoredTransferJob *job = KIO::storedGet( url, KIO::NoReload, 
> KIO::HideProgressInfo );

Why this change? Moving away from https to http seems backward...

REPOSITORY
  R114 Plasma Addons

REVISION DETAIL
  https://phabricator.kde.org/D5729

To: vitali, sebas
Cc: plasma-devel, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, lukas

D5729: #379003: Fix National Geographic POTD provider

[Juri Vitali]

vitali created this revision.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.

REVISION SUMMARY
  The current parsing mechanism to obtain the current NatGeo POTD based on XML 
is broken, this patch proposes an alternative mechanism using 
QRegularExpression library.
  For this method to continue working in the future it is supposed that the web 
page will maintain a sane format, namely that the relevant tag will not be 
broken among many lines, the fields inside the tag will maintain the same 
relative order, and obviously that it will not be obfuscated.

REPOSITORY
  R114 Plasma Addons

REVISION DETAIL
  https://phabricator.kde.org/D5729

AFFECTED FILES
  dataengines/potd/natgeoprovider.cpp

To: vitali, sebas
Cc: plasma-devel, spstarr, progwolff, Zren, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, lukas