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

IXF-Importer: reorder of email content #878

Closed
arnoldnipper opened this issue Nov 14, 2020 · 13 comments
Closed

IXF-Importer: reorder of email content #878

arnoldnipper opened this issue Nov 14, 2020 · 13 comments
Assignees
Labels
AC Admin Committee Data Ownership TF Data Ownership TF related issue Time:Minor Up to 4 hours
Milestone

Comments

@arnoldnipper
Copy link
Contributor

Put the line

You may review and manually accept the IX-F data at https://peeringdb.com//cp/peeringdb_server/ixfmemberdata/nnnn/change/

at the bottom of the email. When quoting that would allow removing internal information with a single action.

@peeringdb/pc, please +1

@arnoldnipper arnoldnipper added AC Admin Committee Data Ownership TF Data Ownership TF related issue labels Nov 14, 2020
@arnoldnipper arnoldnipper added this to the 1 Decide milestone Nov 14, 2020
@arnoldnipper arnoldnipper self-assigned this Nov 14, 2020
@shane-kerr
Copy link

+1

1 similar comment
@ynbrthr
Copy link

ynbrthr commented Nov 16, 2020

+1

@ynbrthr ynbrthr modified the milestones: 1 Decide, 2 Consensus Reached Nov 16, 2020
@peterhelmenstine
Copy link

+1

@ccaputo
Copy link
Contributor

ccaputo commented Nov 20, 2020

With upcoming #875 change, in which the same content goes to PDB admins & ix/network, I recommend the line be moved to the bottom and changed to:

PeeringDB Admins Only: http://localhost/cp/peeringdb_server/ixfmemberdata/nnn/change/

@egfrank
Copy link
Contributor

egfrank commented Nov 27, 2020

Summary

  • Have link at the bottom of IXF emails that states, iff ixf_ixp_member_list_url_visible = "Public"
PeeringDB Admins Only: http://localhost/cp/peeringdb_server/ixfmemberdata/nnn/change/

@arnoldnipper
Copy link
Contributor Author

Only publish the URL if ixf_ixp_member_list_url_visible = "Public", @egfrank

@ccaputo
Copy link
Contributor

ccaputo commented Nov 27, 2020

Only publish the URL if ixf_ixp_member_list_url_visible = "Public", @egfrank

I deleted my previous comment after re-remembering these are two different things. @arnoldnipper, this is different than #875 (comment) which references the the IX-F JSON URL. Rather, here the reference is to an internal PeeringDB URL, not the IX-F JSON URL, thus I believe @egfrank's Summary is correct.

@arnoldnipper
Copy link
Contributor Author

arnoldnipper commented Nov 27, 2020

I deleted my previous comment after re-remembering these are two different things. @arnoldnipper, this is different than #875 (comment) which references the the IX-F JSON URL. Rather, here the reference is to an internal PeeringDB URL, not the IX-F JSON URL, thus I believe @egfrank's Summary is correct.

I know that these are two different things. Though this email is internal, @peeringdb/ac quotes it to send a message to the network. From an @peeringdb/ac pov you could even drop this line. The reason I want to have it at the bottom is to be able to easily remove it when quoting. So, if you move the text to the 2nd last line and obey visibility we could even leave it in.

@ccaputo
Copy link
Contributor

ccaputo commented Nov 27, 2020

I deleted my previous comment after re-remembering these are two different things. @arnoldnipper, this is different than #875 (comment) which references the the IX-F JSON URL. Rather, here the reference is to an internal PeeringDB URL, not the IX-F JSON URL, thus I believe @egfrank's Summary is correct.

I know that these are two different things. Though this email is internal, @peeringdb/ac quotes it to send a message to the network. From an @peeringdb/ac pov you could even drop this line. The reason I want to have it at the bottom is to be able to easily remove it when quoting. So, if you move the text to the 2nd last line and obey visibility we could even leave it in.

The email is not necessarily internal. Once #875 is implemented, the Importer generated Deskpro tickets may CC: external contacts when able.

The "https://peeringdb.com//cp/peeringdb_server/ixfmemberdata/" URL is only present for use by the @peeringdb/ac, as a convenient link should an admin decide to make the suggested change. Users can not use it. Are you saying you don't want it to be included anymore?

@arnoldnipper
Copy link
Contributor Author

he email is not necessarily internal. Once #875 is implemented, the Importer generated Deskpro tickets may CC: external contacts when able.

The "https://peeringdb.com//cp/peeringdb_server/ixfmemberdata/" URL is only present for use by the @peeringdb/ac, as a convenient link should an admin decide to make the suggested change. Users can not use it. Are you saying you don't want it to be included anymore?

Where did I say that? My remarks are only about the IX-F JSON URL.

@ccaputo
Copy link
Contributor

ccaputo commented Nov 27, 2020

he email is not necessarily internal. Once #875 is implemented, the Importer generated Deskpro tickets may CC: external contacts when able.
The "https://peeringdb.com//cp/peeringdb_server/ixfmemberdata/" URL is only present for use by the @peeringdb/ac, as a convenient link should an admin decide to make the suggested change. Users can not use it. Are you saying you don't want it to be included anymore?

Where did I say that? My remarks are only about the IX-F JSON URL.

This ticket was not originally about the IX-F JSON URL, it is about the internal /ixfmemberdata/ URL. Thus when you wrote:

From an @peeringdb/ac pov you could even drop this line.

I took that to be about the line indicated in the first message of this issue:

You may review and manually accept the IX-F data at https://peeringdb.com//cp/peeringdb_server/ixfmemberdata/nnnn/change/

Upon closer reading I see:

So, if you move the text to the 2nd last line and obey visibility we could even leave it in.

and I suppose you mean the IX-F JSON URL line in that context. It would be good to be explicit about the lines we are talking about.

Also, to be very clear, neither of these lines are present in the non-Deskpro emails the Importer sends out. It is not the intent of this issue to add lines to the non-Deskpro emails the Importer sends out. Rather these lines are only in the Deskpro tickets which are created, and which because of #875, will soon CC: networks/IXes upon creation, in addition to being seen by PeeringDB admins.

Thus I am guessing what you mean to propose is:

Summary:

For Deskpro ticket creation (and resulting emails), rearrange lines as follows at the end of the ticket, and factor in "IX-F Member Export URL Visibility" such that when that setting is public:

- IX-F Data: [URL]
- PeeringDB Admins Only: http://peeringdb.com/cp/peeringdb_server/ixfmemberdata/nnn/change/

When not public:

- IX-F Data: Not public
- PeeringDB Admins Only: http://peeringdb.com/cp/peeringdb_server/ixfmemberdata/nnn/change/

@arnoldnipper
Copy link
Contributor Author

Release Notes
This is mainly an internal improvement. And fixes also not disclosing private IXP information to the public.

@vegu
Copy link
Contributor

vegu commented Mar 10, 2021

this was fixed in #875 - closing

@vegu vegu closed this as completed Mar 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AC Admin Committee Data Ownership TF Data Ownership TF related issue Time:Minor Up to 4 hours
Projects
None yet
Development

No branches or pull requests

8 participants