Show Tibet as part of China on map #11930

sgiehl · 2017-08-03T19:52:42Z

This Pull Request is not yet complete.

There might be still some places where old data might be shown as "Unknown" now.

And as old visits won't be changed, segments for showing visits from Tibet might have incorrect results, need to check if there is an easy way to handle that.

fixes #6006

sgiehl · 2017-08-03T21:09:25Z

@Findus23 We need to remove the flag for Tibet as soon as this gets merged in order to get the tests fixed

gameblabla · 2017-08-04T01:20:45Z

I hope this doesn't get merged.

phodal · 2017-08-04T04:31:02Z

@gameblabla Hi, had your used a map? such as Google map, Bing map or Apple map, the last commit about "free tibet" was a joke!

gameblabla · 2017-08-04T04:48:03Z

@phodal, i know them but i do not get your point.
Perhaps it could be done in a different way, keep tibet as a country but mark it as a "disputed territory".

fxzjshm · 2017-08-04T04:52:39Z

@gameblabla Tibet belongs to China, and there is no dispute

gameblabla · 2017-08-04T05:11:43Z

@fxzjshm There's no need to be hostile, i'm trying to find a good compromise. There are people who agree with you and there are people who don't agree. I believe piwik should not be one-sided.

@sgiehl, forgive me but is my idea okay with you ? This might help with the "Unknown" stuff without breaking things. (i admit i know very little php)

phodal · 2017-08-04T05:15:39Z

@gameblabla, did any country recognized Tibet was a country ? America? England ? France? If so, could you list the country name ?

May be for now, India thought Tibet was a country? Because a lot of Indian army was within Chinese borders. Are you come from India, just prepare discuss the disputed territory side of China?

I remember in 1906, according to Convention Between Great Britain and China Respecting Tibet . The Government of China also undertakes not to permit any other foreign State to interfere with the territory or internal administration of Tibet.

So please don't permit interfere with the territory or internal administration of Tibet.

gameblabla · 2017-08-04T06:21:35Z

I'm not sure if this is the place to discuss about this but i will anyway.
@phobal There's India which has troops in Ladakh, which is close to the tibetan border. Britain also recognized it as an autonomous region (as the "administration of Tibet" in the Convention of 1906) until 2008. Before Nixon, the US also recognized Tibet. To this day, it is still disputed by the Dalai-Lama.
So I think it is enough proof that the territory, while indeed under China's control, is still in dispute.
Hence my idea

fengkaijia · 2017-08-04T06:52:45Z

@gameblabla There's no point to discuss historical claims on this. The Taiwan Authority of China claims Outer-Mongolia until 2002, then should we prepare an addition map that has Mongolia inside China? If we don't build map base on the current situation, then I'm sure we need to build hundreds of maps. Like how about a map for the empire that sun never sets? Or a map for the Mongol Empire? No, that's not cool.

gameblabla · 2017-08-04T07:19:42Z

@fengkaijia, the difference is that no one claims the territory for the "Mongol Empire" or "the empire that sun never sets" whereas it's different with Tibet, even now. That's not counting Australia welcoming tibetan refugees.

Ultimately, it's up to piwik's devs to decide if they should go ahead with the merge or not.

fengkaijia · 2017-08-04T07:31:56Z

@gameblabla No no you are mixing up with the refugee concept. The Russian welcomed Edward Snowden, it doesn't mean Russia claimed the US territory. Tibet is a part of China, that's what Australia claims:

The Australian side confirmed the position of successive Australian Governments since 1972 that Australia respects China's sovereignty and territorial integrity, including in relation to Tibet and Xinjiang.

gameblabla · 2017-08-04T07:34:58Z

@fengkaijia, yes you're right, i wasn't denying that. But still, i was pointing out that some people (including tibetans in australia) still do not agree with China on this.

I'll let the devs decide on this.

sgiehl · 2017-08-04T07:45:14Z

There is only one reason why we will change this. We want to show a map that matches other maps of analytics tools like Google Analytics and others. So there won't be any reason in the future to discuss stuff like that. Sure, there might still be some stuff incorrect, but this one can be changed by reverting an old commit, while other changes aren't easily possible without #11929

pengsamkee · 2017-08-04T10:00:50Z

hey，fuck your mum

gameblabla · 2017-08-04T21:12:11Z

I understand the technical reasons.
Still, it is not desirable and having this PR being merged would be disappointing.
The whole point of Piwik was to not become like Google Analytics, if it did,
then what is the point ?

If this PR gets merged, then most likely you'll get more issues about Taiwan and (eventually) Hong Kong.
Do you want to get even more pressure ? Sometimes, it is best not to cooperate to avoid further harassment.

I do wonder what @mattab think of this? He was the one who made the original commit after all.

sgiehl · 2017-08-04T22:05:37Z

@gameblabla I didn't mean Piwik wants to become like Google Analytics.
And I don't want to continue any discussion about that. We aim to use official maps and this is a first step. So if official maps will change we aim to change our maps as well.
And if matomo-org/matomo-map-generator#1 will help to fix our map generator and we generate some new maps, Tibet would be part of China again as well.

gameblabla · 2017-08-05T01:49:13Z

Fine then, i will not insist further. I am considering a fork.
Have a nice day

sgiehl · 2017-08-05T19:41:09Z

Remaining failing tests should be fixed as soon as flag was removed from piwik/icons

fengkaijia · 2017-08-06T06:09:44Z

@sgiehl I just sent a PR matomo-org/matomo-icons#14 to remove the flag.

fengkaijia · 2017-08-06T06:31:54Z

Plus, I wonder if the UserCounty is applied for old visit, plugins/UserCountry/Visitor.php shall also be changed.

And the Archive reader, oh that sounds like a very complicated job.

Findus23 · 2017-08-06T08:16:28Z

@fengkaijia's pull request is merged and reflected in #11904

This reverts commit 4e47c8a.

tsteur · 2017-09-11T19:42:31Z

plugins/UserCountry/API.php

@@ -75,6 +82,14 @@ public function getRegion($idSite, $period, $date, $segment = false)
        $separator = Archiver::LOCATION_SEPARATOR;
        $unk = Visit::UNKNOWN_CODE;

+        // show visits tracked as Tibet as region of China
+        $dataTable->filter('GroupBy', array('label', function($label) {


As those GroupBy filters are quite slow I wonder if we could maybe remove this again in Piwik 4 and somehow mention this in an issue or so? Or maybe we could have first a simple check whether there is actually any row containing this label? Like if($dataTable->getRowFromLabel('1|ti')) { ... groupby filter...}

Same could be done in countries. This would not check lowercase but don't know if we actually have it in different cases in the DB?

Checking if an entry exists before adding the group by filters should work, will test that.
Removing in Piwik 4 might not be that easy. We could add an update script to update all entries in log_visit table (which I would do in next major release only as it might run a bit longer), so we can remove some of the checks for visitor log & profile, but the group by would still be required if the archived reports aren't reprocessed.

It is fine to leave it in there as long as we do the check if table has the row at all.

tsteur · 2017-09-11T19:45:07Z

plugins/UserCountry/functions.php

@@ -22,6 +22,10 @@
 */
 function getFlagFromCode($code)
 {
+    if (strtolower($code) == 'ti') {


Just wondering if this is still needed after moving ti to cn ? Or is it just a fallback?

I see... it might be for visitor log or so

tsteur · 2017-09-11T19:55:13Z

plugins/UserCountryMap/javascripts/visitor-map.js

@@ -1355,10 +1355,10 @@
 $.extend(UserCountryMap, {



Map seems to still work 👍

tsteur

I'd only try to improve the GroupBy to do it only when needed and when the table actually contains such a label, will improve performance a bit as groupBy can take a bit of time (in this case it wouldn't take too long as it would not actually group many, but be still quite a bit faster and take less memory etc). Otherwise good to merge.

sgiehl · 2017-09-12T18:47:38Z

Last changes weren't compatible with Datatable Maps. Just pushed an update. Let's see if all tests pass now and then we can finally merge

tsteur · 2017-09-12T18:51:46Z

👍

fengkaijia · 2017-09-13T04:25:27Z

plugins/UserCountry/API.php

+                    'label',
+                    function ($label) {
+                        if ($label == '1|ti') {
+                            return 'cn';


@sgiehl Thanks for the merge. Should the return value be 14|cn?

sure. I'll fix that typo

Glisse1 · 2017-09-21T19:36:12Z

Chinese pressure is so big that no one can resist it seems.

Say they conquer korea and japan today like they did the massacre in Tibet, next year piwik will update the new map with the new "chinese territories" :)

sgiehl added the Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. label Aug 3, 2017

sgiehl mentioned this pull request Aug 3, 2017

Piwik's Tibet problem in China discussion phodal/piwik-issues#1

Open

phodal mentioned this pull request Aug 4, 2017

Tibet and Taiwan is part of China #11932

Closed

sgiehl mentioned this pull request Aug 4, 2017

Updated / New Matomo Maps #11929

Open

phodal mentioned this pull request Aug 5, 2017

Remove commit 4e47c8 (Remove Tibet from the map) #6006

Closed

fengkaijia mentioned this pull request Aug 6, 2017

Remove Tibetan flag matomo-org/matomo-icons#14

Merged

sgiehl and others added 13 commits September 11, 2017 17:09

Revert "free tibet!"

c301d65

This reverts commit 4e47c8a.

Remove unneeded translations

6c8778b

adjust API

f2a48a4

fix tests

1c061e4

update screenshots

6141d2d

update tests files

5bd5646

update screenshot

504dfda

Do not change detected location

433447b

update tests

aa4a339

Fix continent detection for old records

7d887e9

submodule update

cc9cc11

update screenshots

ab98d13

Cleanup

aa4b0cb

sgiehl force-pushed the mapupdate branch from 6be51bf to aa4b0cb Compare September 11, 2017 15:10

tsteur reviewed Sep 11, 2017

View reviewed changes

plugins/UserCountryMap/javascripts/visitor-map.js

@@ -1355,10 +1355,10 @@

$.extend(UserCountryMap, {

Copy link

Member

tsteur Sep 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Map seems to still work 👍

tsteur requested changes Sep 11, 2017

View reviewed changes

use group by filters only if needed

1dc33fc

tsteur approved these changes Sep 11, 2017

View reviewed changes

make filter work for datatable maps

99b5207

sgiehl merged commit 3cb7169 into 3.x-dev Sep 12, 2017

sgiehl deleted the mapupdate branch September 12, 2017 21:20

fengkaijia reviewed Sep 13, 2017

View reviewed changes

mattab modified the milestones: 3.2.0, 3.1.1 Sep 14, 2017

matomo-org locked and limited conversation to collaborators Sep 21, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show Tibet as part of China on map #11930

Show Tibet as part of China on map #11930

sgiehl commented Aug 3, 2017

sgiehl commented Aug 3, 2017

gameblabla commented Aug 4, 2017

phodal commented Aug 4, 2017

gameblabla commented Aug 4, 2017 •

edited

fxzjshm commented Aug 4, 2017

gameblabla commented Aug 4, 2017 •

edited

phodal commented Aug 4, 2017 •

edited

gameblabla commented Aug 4, 2017

fengkaijia commented Aug 4, 2017 •

edited

gameblabla commented Aug 4, 2017

fengkaijia commented Aug 4, 2017 •

edited

gameblabla commented Aug 4, 2017

sgiehl commented Aug 4, 2017

pengsamkee commented Aug 4, 2017

gameblabla commented Aug 4, 2017 •

edited

sgiehl commented Aug 4, 2017

gameblabla commented Aug 5, 2017

sgiehl commented Aug 5, 2017

fengkaijia commented Aug 6, 2017

fengkaijia commented Aug 6, 2017 •

edited

Findus23 commented Aug 6, 2017

tsteur Sep 11, 2017

sgiehl Sep 11, 2017

tsteur Sep 11, 2017

tsteur Sep 11, 2017 •

edited

tsteur Sep 11, 2017

tsteur Sep 11, 2017

tsteur left a comment

sgiehl commented Sep 12, 2017

tsteur commented Sep 12, 2017

fengkaijia Sep 13, 2017

sgiehl Sep 13, 2017

Glisse1 commented Sep 21, 2017

Show Tibet as part of China on map #11930

Show Tibet as part of China on map #11930

Conversation

sgiehl commented Aug 3, 2017

sgiehl commented Aug 3, 2017

gameblabla commented Aug 4, 2017

phodal commented Aug 4, 2017

gameblabla commented Aug 4, 2017 • edited

fxzjshm commented Aug 4, 2017

gameblabla commented Aug 4, 2017 • edited

phodal commented Aug 4, 2017 • edited

gameblabla commented Aug 4, 2017

fengkaijia commented Aug 4, 2017 • edited

gameblabla commented Aug 4, 2017

fengkaijia commented Aug 4, 2017 • edited

gameblabla commented Aug 4, 2017

sgiehl commented Aug 4, 2017

pengsamkee commented Aug 4, 2017

gameblabla commented Aug 4, 2017 • edited

sgiehl commented Aug 4, 2017

gameblabla commented Aug 5, 2017

sgiehl commented Aug 5, 2017

fengkaijia commented Aug 6, 2017

fengkaijia commented Aug 6, 2017 • edited

Findus23 commented Aug 6, 2017

tsteur Sep 11, 2017

Choose a reason for hiding this comment

sgiehl Sep 11, 2017

Choose a reason for hiding this comment

tsteur Sep 11, 2017

Choose a reason for hiding this comment

tsteur Sep 11, 2017 • edited

Choose a reason for hiding this comment

tsteur Sep 11, 2017

Choose a reason for hiding this comment

tsteur Sep 11, 2017

Choose a reason for hiding this comment

tsteur left a comment

Choose a reason for hiding this comment

sgiehl commented Sep 12, 2017

tsteur commented Sep 12, 2017

fengkaijia Sep 13, 2017

Choose a reason for hiding this comment

sgiehl Sep 13, 2017

Choose a reason for hiding this comment

Glisse1 commented Sep 21, 2017

gameblabla commented Aug 4, 2017 •

edited

gameblabla commented Aug 4, 2017 •

edited

phodal commented Aug 4, 2017 •

edited

fengkaijia commented Aug 4, 2017 •

edited

fengkaijia commented Aug 4, 2017 •

edited

gameblabla commented Aug 4, 2017 •

edited

fengkaijia commented Aug 6, 2017 •

edited

tsteur Sep 11, 2017 •

edited